mbox series

[nft,v2,0/2] Support for variables in map expressions

Message ID 20240403120937.4061434-1-jeremy@azazel.net
Headers show
Series Support for variables in map expressions | expand

Message

Jeremy Sowden April 3, 2024, 12:09 p.m. UTC
The first patch replaces the current assertion failure for invalid
mapping expression in stateful-object statements with an error message.
This brings it in line with map statements.

It is possible to use a variable to initialize a map, which is then used
in a map statement, but if one tries to use the variable directly, nft
rejects it.  The second patch adds support for doing this.

Changes since v1

  * Patch 1 is new.
  * Patch 2 updated to add support for map variables in stateful object
    statements.

Jeremy Sowden (2):
  evaluate: handle invalid mapping expressions in stateful object
    statements gracefully.
  evaluate: add support for variables in map expressions

 src/evaluate.c                                |   7 +-
 .../shell/testcases/maps/0024named_objects_1  |  31 ++++
 .../shell/testcases/maps/anonymous_snat_map_1 |  16 ++
 .../maps/dumps/0024named_objects_1.json-nft   | 147 ++++++++++++++++++
 .../maps/dumps/0024named_objects_1.nft        |  23 +++
 .../maps/dumps/anonymous_snat_map_1.json-nft  |  58 +++++++
 .../maps/dumps/anonymous_snat_map_1.nft       |   5 +
 7 files changed, 285 insertions(+), 2 deletions(-)
 create mode 100755 tests/shell/testcases/maps/0024named_objects_1
 create mode 100755 tests/shell/testcases/maps/anonymous_snat_map_1
 create mode 100644 tests/shell/testcases/maps/dumps/0024named_objects_1.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/0024named_objects_1.nft
 create mode 100644 tests/shell/testcases/maps/dumps/anonymous_snat_map_1.json-nft
 create mode 100644 tests/shell/testcases/maps/dumps/anonymous_snat_map_1.nft

Comments

Pablo Neira Ayuso April 4, 2024, 11:21 a.m. UTC | #1
On Wed, Apr 03, 2024 at 01:09:35PM +0100, Jeremy Sowden wrote:
> The first patch replaces the current assertion failure for invalid
> mapping expression in stateful-object statements with an error message.
> This brings it in line with map statements.
> 
> It is possible to use a variable to initialize a map, which is then used
> in a map statement, but if one tries to use the variable directly, nft
> rejects it.  The second patch adds support for doing this.

Thanks. I can trigger crashes, e.g.

define quota_map = "1.2.3.4"

table ip x {
        chain y {
                quota name ip saddr map $quota_map
        }
}

src/mnl.c:1759:2: runtime error: member access within misaligned address 0x000100000001 for type 'struct expr', which requires 8 byte alignment
0x000100000001: note: pointer points here
<memory cannot be printed>
src/netlink.c:121:10: runtime error: member access within misaligned address 0x000100000001 for type 'const struct expr', which requires 8 byte alignment
0x000100000001: note: pointer points here
<memory cannot be printed>
AddressSanitizer:DEADLYSIGNAL
=================================================================
==150056==ERROR: AddressSanitizer: SEGV on unknown address 0x00009fff8009 (pc 0x7f58e67d8624 bp 0x7ffd57d17eb0 sp 0x7ffd57d17c40 T0)
==150056==The signal is caused by a READ memory access.
    #0 0x7f58e67d8624 in alloc_nftnl_setelem src/netlink.c:121
    #1 0x7f58e67c3d12 in mnl_nft_setelem_batch src/mnl.c:1760
    #2 0x7f58e67c45d9 in mnl_nft_setelem_add src/mnl.c:1805
    #3 0x7f58e687df1e in __do_add_elements src/rule.c:1425
    #4 0x7f58e687e528 in do_add_set src/rule.c:1471
    #5 0x7f58e687e7aa in do_command_add src/rule.c:1491
    #6 0x7f58e688fdb3 in do_command src/rule.c:2599
    #7 0x7f58e679d417 in nft_netlink src/libnftables.c:42
    #8 0x7f58e67a514a in __nft_run_cmd_from_filename src/libnftables.c:729
    #9 0x7f58e67a639c in nft_run_cmd_from_filename src/libnftables.c:807
    #10 0x557c9d25b3b0 in main src/main.c:536
    #11 0x7f58e5846249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #12 0x7f58e5846304 in __libc_start_main_impl ../csu/libc-start.c:360
    #13 0x557c9d258460 in _start (/usr/sbin/nft+0x9460)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV src/netlink.c:121 in alloc_nftnl_setelem
==150056==ABORTING

I think this is lacking more validation.
Jeremy Sowden April 5, 2024, 8:38 p.m. UTC | #2
On 2024-04-04, at 13:21:52 +0200, Pablo Neira Ayuso wrote:
> On Wed, Apr 03, 2024 at 01:09:35PM +0100, Jeremy Sowden wrote:
> > The first patch replaces the current assertion failure for invalid
> > mapping expression in stateful-object statements with an error message.
> > This brings it in line with map statements.
> > 
> > It is possible to use a variable to initialize a map, which is then used
> > in a map statement, but if one tries to use the variable directly, nft
> > rejects it.  The second patch adds support for doing this.
> 
> Thanks. I can trigger crashes, e.g.
> 
> define quota_map = "1.2.3.4"
> 
> table ip x {
>         chain y {
>                 quota name ip saddr map $quota_map
>         }
> }
> 
> src/mnl.c:1759:2: runtime error: member access within misaligned address 0x000100000001 for type 'struct expr', which requires 8 byte alignment
> 0x000100000001: note: pointer points here
> <memory cannot be printed>
> src/netlink.c:121:10: runtime error: member access within misaligned address 0x000100000001 for type 'const struct expr', which requires 8 byte alignment
> 0x000100000001: note: pointer points here
> <memory cannot be printed>
> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==150056==ERROR: AddressSanitizer: SEGV on unknown address 0x00009fff8009 (pc 0x7f58e67d8624 bp 0x7ffd57d17eb0 sp 0x7ffd57d17c40 T0)
> ==150056==The signal is caused by a READ memory access.
>     #0 0x7f58e67d8624 in alloc_nftnl_setelem src/netlink.c:121
>     #1 0x7f58e67c3d12 in mnl_nft_setelem_batch src/mnl.c:1760
>     #2 0x7f58e67c45d9 in mnl_nft_setelem_add src/mnl.c:1805
>     #3 0x7f58e687df1e in __do_add_elements src/rule.c:1425
>     #4 0x7f58e687e528 in do_add_set src/rule.c:1471
>     #5 0x7f58e687e7aa in do_command_add src/rule.c:1491
>     #6 0x7f58e688fdb3 in do_command src/rule.c:2599
>     #7 0x7f58e679d417 in nft_netlink src/libnftables.c:42
>     #8 0x7f58e67a514a in __nft_run_cmd_from_filename src/libnftables.c:729
>     #9 0x7f58e67a639c in nft_run_cmd_from_filename src/libnftables.c:807
>     #10 0x557c9d25b3b0 in main src/main.c:536
>     #11 0x7f58e5846249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>     #12 0x7f58e5846304 in __libc_start_main_impl ../csu/libc-start.c:360
>     #13 0x557c9d258460 in _start (/usr/sbin/nft+0x9460)
> 
> AddressSanitizer can not provide additional info.
> SUMMARY: AddressSanitizer: SEGV src/netlink.c:121 in alloc_nftnl_setelem
> ==150056==ABORTING
> 
> I think this is lacking more validation.

Agreed.  Should have done more testing.  Apologies!  Will follow up.

J.