* [PATCH 5.10.y 1/3] netfilter: nftables: avoid potential overflows on 32bit arches
2021-09-09 14:03 [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Florian Westphal
@ 2021-09-09 14:03 ` Florian Westphal
2021-09-09 14:03 ` [PATCH 5.10.y 2/3] netfilter: nf_tables: initialize set before expression setup Florian Westphal
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2021-09-09 14:03 UTC (permalink / raw)
To: stable; +Cc: netfilter-devel, Eric Dumazet, Pablo Neira Ayuso,
Florian Westphal
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;
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5.10.y 2/3] netfilter: nf_tables: initialize set before expression setup
2021-09-09 14:03 [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Florian Westphal
2021-09-09 14:03 ` [PATCH 5.10.y 1/3] netfilter: nftables: avoid potential overflows on 32bit arches Florian Westphal
@ 2021-09-09 14:03 ` Florian Westphal
2021-09-09 14:03 ` [PATCH 5.10.y 3/3] netfilter: nftables: clone set element expression template Florian Westphal
2021-09-09 14:52 ` [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Greg KH
3 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2021-09-09 14:03 UTC (permalink / raw)
To: stable
Cc: netfilter-devel, Pablo Neira Ayuso, syzbot+ce96ca2b1d0b37c6422d,
Florian Westphal
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);
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5.10.y 3/3] netfilter: nftables: clone set element expression template
2021-09-09 14:03 [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Florian Westphal
2021-09-09 14:03 ` [PATCH 5.10.y 1/3] netfilter: nftables: avoid potential overflows on 32bit arches Florian Westphal
2021-09-09 14:03 ` [PATCH 5.10.y 2/3] netfilter: nf_tables: initialize set before expression setup Florian Westphal
@ 2021-09-09 14:03 ` Florian Westphal
2021-09-09 14:52 ` [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Greg KH
3 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2021-09-09 14:03 UTC (permalink / raw)
To: stable
Cc: netfilter-devel, Pablo Neira Ayuso, Laura Garcia Liebana,
Florian Westphal
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--;
--
2.32.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y
2021-09-09 14:03 [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Florian Westphal
` (2 preceding siblings ...)
2021-09-09 14:03 ` [PATCH 5.10.y 3/3] netfilter: nftables: clone set element expression template Florian Westphal
@ 2021-09-09 14:52 ` Greg KH
2021-09-11 9:39 ` Salvatore Bonaccorso
3 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2021-09-09 14:52 UTC (permalink / raw)
To: Florian Westphal; +Cc: stable, netfilter-devel
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:
> 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(-)
>
> --
> 2.32.0
>
All now queued up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y
2021-09-09 14:52 ` [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y Greg KH
@ 2021-09-11 9:39 ` Salvatore Bonaccorso
2021-09-11 12:06 ` Florian Westphal
0 siblings, 1 reply; 7+ messages in thread
From: Salvatore Bonaccorso @ 2021-09-11 9:39 UTC (permalink / raw)
To: Greg KH; +Cc: Florian Westphal, stable, netfilter-devel
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:
> > 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(-)
> >
> > --
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5.10.y 0/3] netfilter: nf_tables fixes for 5.10.y
2021-09-11 9:39 ` Salvatore Bonaccorso
@ 2021-09-11 12:06 ` Florian Westphal
0 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2021-09-11 12:06 UTC (permalink / raw)
To: Salvatore Bonaccorso; +Cc: Greg KH, Florian Westphal, stable, netfilter-devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread