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(-)
From: Eric Dumazet edumazet@google.com
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.
Fixes: 0ed6389c483d ("netfilter: nf_tables: rename set implementations") Signed-off-by: Eric Dumazet edumazet@google.com Signed-off-by: Pablo Neira Ayuso pablo@netfilter.org Signed-off-by: Florian Westphal fw@strlen.de --- net/netfilter/nf_tables_api.c | 7 +++++-- net/netfilter/nft_set_hash.c | 10 +++++----- 2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index e34d05cc5754..947d52cff582 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -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;
diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index d7083bcb20e8..858c8d4d659a 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -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 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;
From: Pablo Neira Ayuso pablo@netfilter.org
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]
Reported-by: syzbot+ce96ca2b1d0b37c6422d@syzkaller.appspotmail.com Fixes: 65038428b2c6 ("netfilter: nf_tables: allow to specify stateful expression in set definition") Signed-off-by: Pablo Neira Ayuso pablo@netfilter.org Signed-off-by: Florian Westphal fw@strlen.de --- net/netfilter/nf_tables_api.c | 46 ++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 947d52cff582..3942a29413a4 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -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);
From: Pablo Neira Ayuso pablo@netfilter.org
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]
Reported-by: Laura Garcia Liebana nevola@gmail.com Fixes: 409444522976 ("netfilter: nf_tables: add elements with stateful expressions") Signed-off-by: Pablo Neira Ayuso pablo@netfilter.org Signed-off-by: Florian Westphal fw@strlen.de --- net/netfilter/nf_tables_api.c | 36 +++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 3942a29413a4..2b5f97e1d40b 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -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--;
On Thu, Sep 09, 2021 at 04:03:34PM +0200, Florian Westphal wrote:
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:
- 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.
- Tested the 'conncount' feature (its affected by last patch). Worked as designed.
- 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(-)
-- 2.32.0
All now queued up, thanks!
greg k-h
Hi Greg,
On Thu, Sep 09, 2021 at 04:52:09PM +0200, Greg KH wrote:
On Thu, Sep 09, 2021 at 04:03:34PM +0200, Florian Westphal wrote:
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:
- 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.
- Tested the 'conncount' feature (its affected by last patch). Worked as designed.
- 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(-)
-- 2.32.0
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 carnil@debian.org
Regards, Salvatore
Salvatore Bonaccorso carnil@debian.org wrote:
On Thu, Sep 09, 2021 at 04:52:09PM +0200, Greg KH wrote:
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 carnil@debian.org
Thanks for testing!
Please let us know if anything else in netfilter territory is not working as expected.
linux-stable-mirror@lists.linaro.org
-
Florian Westphal -
Greg KH -
Salvatore Bonaccorso