>From the original report:
"""
If the catchall element is gc'd when the pipapo set is removed, the element
can be deactivated twice.
When a set is deleted, the nft_map_deactivate() is called to deactivate the
data of the set elements [1].
static int nft_delset(const struct nft_ctx *ctx, struct nft_set *set)
{
int err;
err = nft_trans_set_add(ctx, NFT_MSG_DELSET, set);
if (err < 0)
return err;
if (set->flags & (NFT_SET_MAP | NFT_SET_OBJECT))
nft_map_deactivate(ctx, set); // [1]
nft_deactivate_next(ctx->net, set);
nft_use_dec(&ctx->table->use);
return err;
}
Then nft_set_commit_update() is called in the nf_tables_commit() and the
pipapo set's commit function is called [2].
static void nft_set_commit_update(struct list_head *set_update_list)
{
struct nft_set *set, *next;
list_for_each_entry_safe(set, next, set_update_list, pending_update) {
list_del_init(&set->pending_update);
if (!set->ops->commit)
continue;
set->ops->commit(set); // [2]
}
}
In the nft_pipapo_commit(), nft_trans_gc_catchall_sync() is called and
nft_setelem_data_deactivate() is called for expired elements [3].
struct nft_trans_gc *nft_trans_gc_catchall_sync(struct nft_trans_gc *gc)
{
struct nft_set_elem_catchall *catchall, *next;
const struct nft_set *set = gc->set;
struct nft_set_elem elem;
struct nft_set_ext *ext;
WARN_ON_ONCE(!lockdep_commit_lock_is_held(gc->net));
list_for_each_entry_safe(catchall, next, &set->catchall_list, list) {
ext = nft_set_elem_ext(set, catchall->elem);
if (!nft_set_elem_expired(ext))
continue;
gc = nft_trans_gc_queue_sync(gc, GFP_KERNEL);
if (!gc)
return NULL;
memset(&elem, 0, sizeof(elem));
elem.priv = catchall->elem;
nft_setelem_data_deactivate(gc->net, gc->set, &elem); // [3]
nft_setelem_catchall_destroy(catchall);
nft_trans_gc_elem_add(gc, elem.priv);
}
return gc;
}
This element is deactivated by calling the nft_map_deactivate() in the
nft_delset() [1], so the data deactivation is done twice. This causes a UAF
on an NFT_CHAIN object or NFT_OBJECT object.
In nft_data_release, chain->use is underflowed, and the following log is
printed.
[ 3.709887] ------------[ cut here ]------------
[ 3.710046] WARNING: CPU: 0 PID: 345 at
include/net/netfilter/nf_tables.h:1190 nft_data_release+0x5d/0x70
[ 3.710385] Modules linked in:
[ 3.710495] CPU: 0 PID: 345 Comm: poc Not tainted 6.7.0-rc6 #251
[ 3.710701] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.13.0-1ubuntu1.1 04/01/2014
[ 3.711005] RIP: 0010:nft_data_release+0x5d/0x70
[ 3.711172] Code: 06 5b c3 cc cc cc cc 48 8d 7b 08 e8 0d f8 ce fe 48 8b
5b 08 48 8d 7b 50 e8 c0 f6 ce fe 8b 43 50 8d 50 ff 89 53 50 85 c0 75 d7
<0f> 0b 5b c3 cc cc cc cc 66 66 2e 0f 1f 84 00 00 00 00 00 90 9
[ 3.711784] RSP: 0018:ffff888107437328 EFLAGS: 00010246
[ 3.711962] RAX: 0000000000000000 RBX: ffff88810031b200 RCX:
dffffc0000000000
[ 3.712206] RDX: 00000000ffffffff RSI: 00000000ffffff00 RDI:
ffff88810031b250
[ 3.712481] RBP: ffff888104b68000 R08: ffffffffae99d250 R09:
ffffed102096d005
[ 3.712722] R10: ffffed102096d004 R11: ffff888104b68023 R12:
0000000000000020
[ 3.712968] R13: ffff888104b680e0 R14: ffff888104b68000 R15:
ffff8881061d4018
[ 3.713234] FS: 000000000082e880(0000) GS:ffff88811b200000(0000)
knlGS:0000000000000000
[ 3.713523] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3.713730] CR2: 0000000000839000 CR3: 00000001090ce000 CR4:
0000000000750ef0
[ 3.714072] PKRU: 55555554
[ 3.714213] Call Trace:
[ 3.714331] <TASK>
[ 3.714432] ? __warn+0xa1/0x1c0
[ 3.714588] ? nft_data_release+0x5d/0x70
[ 3.714783] ? report_bug+0x265/0x270
[ 3.714953] ? handle_bug+0x42/0x80
[ 3.715093] ? exc_invalid_op+0x14/0x50
[ 3.715238] ? asm_exc_invalid_op+0x16/0x20
[ 3.715399] ? nft_data_release+0x50/0x70
[ 3.715553] ? nft_data_release+0x5d/0x70
[ 3.715708] ? nft_data_release+0x50/0x70
[ 3.715864] nft_setelem_data_deactivate+0x67/0xb0
[ 3.716051] nft_trans_gc_catchall_sync+0xae/0x200
[ 3.716246] pipapo_gc+0x356/0x3f0
[ 3.716392] ? __pfx_pipapo_gc+0x10/0x10
[ 3.716565] ? _raw_spin_lock+0x87/0xe0
[ 3.716728] ? __pfx__raw_spin_lock+0x10/0x10
[ 3.716910] ? __rcu_read_unlock+0x4e/0x260
[ 3.717087] ? kasan_quarantine_put+0x55/0x180
[ 3.717273] nft_pipapo_commit+0x7b/0x140
[ 3.717423] nf_tables_commit+0x10a7/0x2200
"""
The original patch says expired catchall elements are not deactivated,
but nft_setelem_data_deactivate is called for all map objects. So, only
call nft_setelem_data_deactivate for non map objects.
Fixes: 93995bf4af2c ("netfilter: nf_tables: remove catchall element in GC sync path")
Reported-by: Kevin Rich <kevinrich1337@xxxxxxxxx>
Signed-off-by: Wander Lairson Costa <wander@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
net/netfilter/nf_tables_api.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c5c17c6e80ed..2defdf56484f 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -9728,7 +9728,11 @@ struct nft_trans_gc *nft_trans_gc_catchall_sync(struct nft_trans_gc *gc)
return NULL;
elem_priv = catchall->elem;
- nft_setelem_data_deactivate(gc->net, gc->set, elem_priv);
+
+ /* For map objects, nft_map_catchall_deactivate has already been called. */
+ if (!(set->flags & (NFT_SET_MAP | NFT_SET_OBJECT)))
+ nft_setelem_data_deactivate(gc->net, gc->set, elem_priv);
+
nft_setelem_catchall_destroy(catchall);
nft_trans_gc_elem_add(gc, elem_priv);
}
--
2.43.0
