| Message ID | 20230906162525.11079-7-fw@strlen.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9b5ba5c9c5109bf89dc64a3f4734bd125d1ce52e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/6] netfilter: nftables: exthdr: fix 4-byte stack OOB write | expand |
On Wed, Sep 06, 2023 at 06:25:12PM +0200, Florian Westphal wrote: > From: Pablo Neira Ayuso <pablo@netfilter.org> > > Deliver audit log from __nf_tables_dump_rules(), table dereference at > the end of the table list loop might point to the list head, leading to > this crash. There are a few issues with this patch, can we please drop it from this MR for now? Thanks, Phil
Phil Sutter <phil@nwl.cc> wrote: > On Wed, Sep 06, 2023 at 06:25:12PM +0200, Florian Westphal wrote: > > From: Pablo Neira Ayuso <pablo@netfilter.org> > > > > Deliver audit log from __nf_tables_dump_rules(), table dereference at > > the end of the table list loop might point to the list head, leading to > > this crash. > > There are a few issues with this patch, can we please drop it from this > MR for now? If this were a change that *adds* a kernel crash, then, sure. But this fixes a crash, so I see no reason to keep it back. Please do an incremental followup instead. Thanks.
On Thu, Sep 07, 2023 at 12:41:37AM +0200, Florian Westphal wrote: > Phil Sutter <phil@nwl.cc> wrote: > > On Wed, Sep 06, 2023 at 06:25:12PM +0200, Florian Westphal wrote: > > > From: Pablo Neira Ayuso <pablo@netfilter.org> > > > > > > Deliver audit log from __nf_tables_dump_rules(), table dereference at > > > the end of the table list loop might point to the list head, leading to > > > this crash. > > > > There are a few issues with this patch, can we please drop it from this > > MR for now? > > If this were a change that *adds* a kernel crash, then, sure. > But this fixes a crash, so I see no reason to keep it back. > > Please do an incremental followup instead. ACK, I'll do that instead. Thanks, Phil
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 2c81cee858d6..e429ebba74b3 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3480,6 +3480,10 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, cont_skip: (*idx)++; } + + if (reset && *idx) + audit_log_rule_reset(table, cb->seq, *idx); + return 0; } @@ -3540,9 +3544,6 @@ static int nf_tables_dump_rules(struct sk_buff *skb, done: rcu_read_unlock(); - if (reset && idx > cb->args[0]) - audit_log_rule_reset(table, cb->seq, idx - cb->args[0]); - cb->args[0] = idx; return skb->len; } @@ -5760,8 +5761,6 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) if (!args.iter.err && args.iter.count == cb->args[0]) args.iter.err = nft_set_catchall_dump(net, skb, set, reset, cb->seq); - rcu_read_unlock(); - nla_nest_end(skb, nest); nlmsg_end(skb, nlh); @@ -5769,6 +5768,8 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb) audit_log_nft_set_reset(table, cb->seq, args.iter.count - args.iter.skip); + rcu_read_unlock(); + if (args.iter.err && args.iter.err != -EMSGSIZE) return args.iter.err; if (args.iter.count == cb->args[0])