netfilter: nf_tables fixes for 5.10.y
Subsystems (2)
Florian Westphal
maintainercover netfilter: nf_tables fixes for 5.10.y
Hello,
please consider applying these nf_tables fixes to the 5.10.y tree. These patches had to mangled to make them apply to 5.10.y.
I've done the follwoing tests in a kasan/kmemleak enabled vm:
1. run upstream nft python/shell tests.
Without patch 2 and 3 doing so results in kernel crash. Some tests fail but afaics those are expected to fail on 5.10 due to lack of feature being tested.
2. Tested the 'conncount' feature (its affected by last patch). Worked as designed.
3. ran nftables related kernel self tests.
No kmemleak or kasan splats were seen.
Eric Dumazet (1):
netfilter: nftables: avoid potential overflows on 32bit arches
Pablo Neira Ayuso (2):
netfilter: nf_tables: initialize set before expression setup
netfilter: nftables: clone set element expression template
net/netfilter/nf_tables_api.c | 89 ++++++++++++++++++++++------------- net/netfilter/nft_set_hash.c | 10 ++-- 2 files changed, 62 insertions(+), 37 deletions(-)
Signature · 1 line
2.32.0
Florian Westphal
maintainerPATCH 1/3 netfilter: nftables: avoid potential overflows on 32bit arches
From: Eric Dumazet <[email protected]>
commit 6c8774a94e6ad26f29ef103c8671f55c255c6201 upstream.
User space could ask for very large hash tables, we need to make sure our size computations wont overflow.
nf_tables_newset() needs to double check the u64 size will fit into size_t field.
--- net/netfilter/nf_tables_api.c | 7 +++++-- net/netfilter/nft_set_hash.c | 10 +++++----- 2 files changed, 10 insertions(+), 7 deletions(-)
net/netfilter/nf_tables_api.c +5 -2
@@ -4115,6 +4115,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,struct nft_table *table;struct nft_set *set;struct nft_ctx ctx; +size_t alloc_size;char *name;u64 size;u64 timeout;
@@ -4263,8 +4264,10 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, size =0 ;if (ops->privsize !=NULL ) size = ops->privsize (nla, &desc); - - set =kvzalloc (sizeof (*set) + size + udlen,GFP_KERNEL ); + alloc_size =sizeof (*set) + size + udlen; +if (alloc_size < size) +return -ENOMEM ; + set =kvzalloc (alloc_size,GFP_KERNEL );if (!set)return -ENOMEM ;
net/netfilter/nft_set_hash.c +5 -5
@@ -604,7 +604,7 @@ static u64 nft_hash_privsize(const struct nlattr * const nla[], const struct nft_set_desc *desc) {return sizeof (struct nft_hash ) + -nft_hash_buckets (desc->size) *sizeof (struct hlist_head ); + (u64 )nft_hash_buckets (desc->size) *sizeof (struct hlist_head ); } static int nft_hash_init(const struct nft_set *set,
@@ -644,8 +644,8 @@ static bool nft_hash_estimate(const struct nft_set_desc *desc, u32 features,return false; est->size =sizeof (struct nft_hash ) + -nft_hash_buckets (desc->size) *sizeof (struct hlist_head ) + - desc->size *sizeof (struct nft_hash_elem ); + (u64 )nft_hash_buckets (desc->size) *sizeof (struct hlist_head ) + + (u64 )desc->size *sizeof (struct nft_hash_elem ); est->lookup =NFT_SET_CLASS_O_1 ; est->space =NFT_SET_CLASS_O_N ;
@@ -662,8 +662,8 @@ static bool nft_hash_fast_estimate(const struct nft_set_desc *desc, u32 featuresreturn false; est->size =sizeof (struct nft_hash ) + -nft_hash_buckets (desc->size) *sizeof (struct hlist_head ) + - desc->size *sizeof (struct nft_hash_elem ); + (u64 )nft_hash_buckets (desc->size) *sizeof (struct hlist_head ) + + (u64 )desc->size *sizeof (struct nft_hash_elem ); est->lookup =NFT_SET_CLASS_O_1 ; est->space =NFT_SET_CLASS_O_N ;
Signature · 1 line
2.32.0
Florian Westphal
maintainerPATCH 2/3 netfilter: nf_tables: initialize set before expression setup
In reply to Florian WestphalFrom: Pablo Neira Ayuso <[email protected]>
commit ad9f151e560b016b6ad3280b48e42fa11e1a5440 upstream.
nft_set_elem_expr_alloc() needs an initialized set if expression sets on the NFT_EXPR_GC flag. Move set fields initialization before expression setup.
[4512935.019450] ================================================================== [4512935.019456] BUG: KASAN: null-ptr-deref in nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables] [4512935.019487] Read of size 8 at addr 0000000000000070 by task nft/23532 [4512935.019494] CPU: 1 PID: 23532 Comm: nft Not tainted 5.12.0-rc4+ #48 [...] [4512935.019502] Call Trace: [4512935.019505] dump_stack+0x89/0xb4 [4512935.019512] ? nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables] [4512935.019536] ? nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables] [4512935.019560] kasan_report.cold.12+0x5f/0xd8 [4512935.019566] ? nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables] [4512935.019590] nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables] [4512935.019615] nf_tables_newset+0xc7f/0x1460 [nf_tables]
--- net/netfilter/nf_tables_api.c | 46 ++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 22 deletions(-)
net/netfilter/nf_tables_api.c +24 -22
@@ -4280,15 +4280,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, err =nf_tables_set_alloc_name (&ctx, set, name);kfree (name);if (err <0 ) - goto err_set_alloc_name; - -if (nla[NFTA_SET_EXPR ]) { - expr =nft_set_elem_expr_alloc (&ctx, set, nla[NFTA_SET_EXPR ]); -if (IS_ERR (expr)) { - err =PTR_ERR (expr); - goto err_set_alloc_name; - } - } + goto err_set_name; udata =NULL ;if (udlen) {
@@ -4299,21 +4291,19 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,INIT_LIST_HEAD (&set->bindings); set->table = table;write_pnet (&set->net, net); - set->ops = ops; + set->ops = ops; set->ktype = ktype; - set->klen = desc.klen; + set->klen = desc.klen; set->dtype = dtype; set->objtype = objtype; - set->dlen = desc.dlen; - set->expr = expr; + set->dlen = desc.dlen; set->flags = flags; - set->size = desc.size; + set->size = desc.size; set->policy = policy; - set->udlen = udlen; - set->udata = udata; + set->udlen = udlen; + set->udata = udata; set->timeout = timeout; set->gc_int = gc_int; - set->handle =nf_tables_alloc_handle (table); set->field_count = desc.field_count;for (i =0 ; i < desc.field_count; i++)
@@ -4323,20 +4313,32 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,if (err <0 ) goto err_set_init; +if (nla[NFTA_SET_EXPR ]) { + expr =nft_set_elem_expr_alloc (&ctx, set, nla[NFTA_SET_EXPR ]); +if (IS_ERR (expr)) { + err =PTR_ERR (expr); + goto err_set_expr_alloc; + } + + set->expr = expr; + } + + set->handle =nf_tables_alloc_handle (table); + err =nft_trans_set_add (&ctx,NFT_MSG_NEWSET , set);if (err <0 ) - goto err_set_trans; + goto err_set_expr_alloc;list_add_tail_rcu (&set->list, &table->sets); table->use++;return 0 ; -err_set_trans: +err_set_expr_alloc: +if (set->expr) +nft_expr_destroy (&ctx, set->expr); + ops->destroy (set); err_set_init: -if (expr) -nft_expr_destroy (&ctx, expr); -err_set_alloc_name:kfree (set->name); err_set_name:kvfree (set);
Signature · 1 line
2.32.0
Florian Westphal
maintainerPATCH 3/3 netfilter: nftables: clone set element expression template
In reply to Florian WestphalFrom: Pablo Neira Ayuso <[email protected]>
commit 4d8f9065830e526c83199186c5f56a6514f457d2 upstream.
memcpy() breaks when using connlimit in set elements. Use nft_expr_clone() to initialize the connlimit expression list, otherwise connlimit garbage collector crashes when walking on the list head copy.
[ 493.064656] Workqueue: events_power_efficient nft_rhash_gc [nf_tables] [ 493.064685] RIP: 0010:find_or_evict+0x5a/0x90 [nf_conncount] [ 493.064694] Code: 2b 43 40 83 f8 01 77 0d 48 c7 c0 f5 ff ff ff 44 39 63 3c 75 df 83 6d 18 01 48 8b 43 08 48 89 de 48 8b 13 48 8b 3d ee 2f 00 00 <48> 89 42 08 48 89 10 48 b8 00 01 00 00 00 00 ad de 48 89 03 48 83 [ 493.064699] RSP: 0018:ffffc90000417dc0 EFLAGS: 00010297 [ 493.064704] RAX: 0000000000000000 RBX: ffff888134f38410 RCX: 0000000000000000 [ 493.064708] RDX: 0000000000000000 RSI: ffff888134f38410 RDI: ffff888100060cc0 [ 493.064711] RBP: ffff88812ce594a8 R08: ffff888134f38438 R09: 00000000ebb9025c [ 493.064714] R10: ffffffff8219f838 R11: 0000000000000017 R12: 0000000000000001 [ 493.064718] R13: ffffffff82146740 R14: ffff888134f38410 R15: 0000000000000000 [ 493.064721] FS: 0000000000000000(0000) GS:ffff88840e440000(0000) knlGS:0000000000000000 [ 493.064725] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 493.064729] CR2: 0000000000000008 CR3: 00000001330aa002 CR4: 00000000001706e0 [ 493.064733] Call Trace: [ 493.064737] nf_conncount_gc_list+0x8f/0x150 [nf_conncount] [ 493.064746] nft_rhash_gc+0x106/0x390 [nf_tables]
--- net/netfilter/nf_tables_api.c | 36 +++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-)
net/netfilter/nf_tables_api.c +28 -8
@@ -5150,6 +5150,24 @@ static void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,kfree (elem); } +static int nft_set_elem_expr_setup (struct nft_ctx *ctx, +const struct nft_set_ext *ext, +struct nft_expr *expr) +{ +struct nft_expr *elem_expr =nft_set_ext_expr (ext); +int err; + +if (expr ==NULL ) +return 0 ; + + err =nft_expr_clone (elem_expr, expr); +if (err <0 ) +return -ENOMEM ; + +nft_expr_destroy (ctx, expr); +return 0 ; +} +static int nft_add_set_elem (struct nft_ctx *ctx,struct nft_set *set,const struct nlattr *attr,u32 nlmsg_flags) {
@@ -5352,15 +5370,17 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, *nft_set_ext_obj (ext) = obj; obj->use++; } -if (expr) { -memcpy (nft_set_ext_expr (ext), expr, expr->ops->size); -kfree (expr); - expr =NULL ; - } + + err =nft_set_elem_expr_setup (ctx, ext, expr); +if (err <0 ) + goto err_elem_expr; + expr =NULL ; trans =nft_trans_elem_alloc (ctx,NFT_MSG_NEWSETELEM , set); -if (trans ==NULL ) - goto err_trans; +if (trans ==NULL ) { + err = -ENOMEM ; + goto err_elem_expr; + } ext->genmask =nft_genmask_cur (ctx->net) |NFT_SET_ELEM_BUSY_MASK ; err = set->ops->insert (ctx->net, set, &elem, &ext2);
@@ -5404,7 +5424,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set, set->ops->remove (ctx->net, set, &elem); err_element_clash:kfree (trans); -err_trans: +err_elem_expr:if (obj) obj->use--;
Signature · 1 line
2.32.0
Greg KH
maintainerQuoted 19 lines · Florian Westphal
Hello,
please consider applying these
nf_tablesfixes to the 5.10.y tree. These patches had to mangled to make them apply to 5.10.y.I've done the follwoing tests in a kasan/kmemleak enabled vm:
1. run upstream nft python/shell tests.
Without patch 2 and 3 doing so results in kernel crash. Some tests fail but afaics those are expected to fail on 5.10 due to lack of feature being tested.
2. Tested the 'conncount' feature (its affected by last patch). Worked as designed.
3. ran nftables related kernel self tests.No kmemleak or kasan splats were seen.
Eric Dumazet (1):
netfilter: nftables: avoid potential overflows on 32bit arches
Quoted 11 lines
Pablo Neira Ayuso (2):
netfilter:nf_tables: initialize set before expression setup
netfilter: nftables: clone set element expression templatenet/netfilter/nf_tables_api.c | 89 ++++++++++++++++++++++------------- net/netfilter/nft_set_hash.c | 10 ++-- 2 files changed, 62 insertions(+), 37 deletions(-)Signature · 1 line
2.32.0
All now queued up, thanks!
greg k-h
Salvatore Bonaccorso
Tested-byHi Greg,
Quoted 28 lines · Greg KH
Quoted 27 lines · Florian Westphal
Hello,
please consider applying these
nf_tablesfixes to the 5.10.y tree. These patches had to mangled to make them apply to 5.10.y.I've done the follwoing tests in a kasan/kmemleak enabled vm:
1. run upstream nft python/shell tests.
Without patch 2 and 3 doing so results in kernel crash. Some tests fail but afaics those are expected to fail on 5.10 due to lack of feature being tested.
2. Tested the 'conncount' feature (its affected by last patch). Worked as designed.
3. ran nftables related kernel self tests.No kmemleak or kasan splats were seen.
Eric Dumazet (1):
netfilter: nftables: avoid potential overflows on 32bit archesPablo Neira Ayuso (2):
netfilter:nf_tables: initialize set before expression setup
netfilter: nftables: clone set element expression templatenet/netfilter/nf_tables_api.c | 89 ++++++++++++++++++++++------------- net/netfilter/nft_set_hash.c | 10 ++-- 2 files changed, 62 insertions(+), 37 deletions(-)
Quoted 5 lines
Quoted 3 lines
Signature · 1 line
2.32.0All now queued up, thanks!
Florian, thank you! My query originated from a bugreport in Debian triggering the issue with the 5.10.y kernels used.
Not really needed here as Greg already queued up but:
Regards,
Salvatore
Florian Westphal
maintainerQuoted 9 lines · Salvatore Bonaccorso
Quoted 1 line · Greg KH
All now queued up, thanks!
Florian, thank you! My query originated from a bugreport in Debian triggering the issue with the 5.10.y kernels used.
Not really needed here as Greg already queued up but:
Tested-by Salvatore Bonaccorso <[email protected]>
Thanks for testing!
Please let us know if anything else in netfilter territory is not working as expected.