Skip to content

Commit 9f6958b

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: nf_tables: unconditionally flush pending work before notifier
syzbot reports: KASAN: slab-uaf in nft_ctx_update include/net/netfilter/nf_tables.h:1831 KASAN: slab-uaf in nft_commit_release net/netfilter/nf_tables_api.c:9530 KASAN: slab-uaf int nf_tables_trans_destroy_work+0x152b/0x1750 net/netfilter/nf_tables_api.c:9597 Read of size 2 at addr ffff88802b0051c4 by task kworker/1:1/45 [..] Workqueue: events nf_tables_trans_destroy_work Call Trace: nft_ctx_update include/net/netfilter/nf_tables.h:1831 [inline] nft_commit_release net/netfilter/nf_tables_api.c:9530 [inline] nf_tables_trans_destroy_work+0x152b/0x1750 net/netfilter/nf_tables_api.c:9597 Problem is that the notifier does a conditional flush, but its possible that the table-to-be-removed is still referenced by transactions being processed by the worker, so we need to flush unconditionally. We could make the flush_work depend on whether we found a table to delete in nf-next to avoid the flush for most cases. AFAICS this problem is only exposed in nf-next, with commit e169285 ("netfilter: nf_tables: do not store nft_ctx in transaction objects"), with this commit applied there is an unconditional fetch of table->family which is whats triggering the above splat. Fixes: 2c9f029 ("netfilter: nf_tables: flush pending destroy work before netlink notifier") Reported-and-tested-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=4fd66a69358fc15ae2ad Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 8905a2c commit 9f6958b

File tree

1 file changed

+1
-2
lines changed

1 file changed

+1
-2
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11483,8 +11483,7 @@ static int nft_rcv_nl_event(struct notifier_block *this, unsigned long event,
1148311483

1148411484
gc_seq = nft_gc_seq_begin(nft_net);
1148511485

11486-
if (!list_empty(&nf_tables_destroy_list))
11487-
nf_tables_trans_destroy_flush_work();
11486+
nf_tables_trans_destroy_flush_work();
1148811487
again:
1148911488
list_for_each_entry(table, &nft_net->tables, list) {
1149011489
if (nft_table_has_owner(table) &&

0 commit comments

Comments
 (0)