Skip to content

Commit 3d64c3d

Browse files
author
Paolo Abeni
committed
Merge tag 'nf-24-12-11' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf
Pablo Neira Ayuso says: ==================== Netfilter fixes for net The following patchset contains Netfilter fixes for net: 1) Fix bogus test reports in rpath.sh selftest by adding permanent neighbor entries, from Phil Sutter. 2) Lockdep reports possible ABBA deadlock in xt_IDLETIMER, fix it by removing sysfs out of the mutex section, also from Phil Sutter. 3) It is illegal to release basechain via RCU callback, for several reasons. Keep it simple and safe by calling synchronize_rcu() instead. This is a partially reverting a botched recent attempt of me to fix this basechain release path on netdevice removal. From Florian Westphal. netfilter pull request 24-12-11 * tag 'nf-24-12-11' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf: netfilter: nf_tables: do not defer rule destruction via call_rcu netfilter: IDLETIMER: Fix for possible ABBA deadlock selftests: netfilter: Stabilize rpath.sh ==================== Link: https://patch.msgid.link/20241211230130.176937-1-pablo@netfilter.org Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2 parents 9871284 + b04df3d commit 3d64c3d

File tree

4 files changed

+59
-47
lines changed

4 files changed

+59
-47
lines changed

‎include/net/netfilter/nf_tables.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,6 @@ struct nft_rule_blob {
11031103
* @name: name of the chain
11041104
* @udlen: user data length
11051105
* @udata: user data in the chain
1106-
* @rcu_head: rcu head for deferred release
11071106
* @blob_next: rule blob pointer to the next in the chain
11081107
*/
11091108
struct nft_chain {
@@ -1121,7 +1120,6 @@ struct nft_chain {
11211120
char *name;
11221121
u16 udlen;
11231122
u8 *udata;
1124-
struct rcu_head rcu_head;
11251123

11261124
/* Only used during control plane commit phase: */
11271125
struct nft_rule_blob *blob_next;
@@ -1265,7 +1263,6 @@ static inline void nft_use_inc_restore(u32 *use)
12651263
* @sets: sets in the table
12661264
* @objects: stateful objects in the table
12671265
* @flowtables: flow tables in the table
1268-
* @net: netnamespace this table belongs to
12691266
* @hgenerator: handle generator state
12701267
* @handle: table handle
12711268
* @use: number of chain references to this table
@@ -1285,7 +1282,6 @@ struct nft_table {
12851282
struct list_head sets;
12861283
struct list_head objects;
12871284
struct list_head flowtables;
1288-
possible_net_t net;
12891285
u64 hgenerator;
12901286
u64 handle;
12911287
u32 use;

‎net/netfilter/nf_tables_api.c‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,7 +1596,6 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
15961596
INIT_LIST_HEAD(&table->sets);
15971597
INIT_LIST_HEAD(&table->objects);
15981598
INIT_LIST_HEAD(&table->flowtables);
1599-
write_pnet(&table->net, net);
16001599
table->family = family;
16011600
table->flags = flags;
16021601
table->handle = ++nft_net->table_handle;
@@ -3987,8 +3986,11 @@ void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule)
39873986
kfree(rule);
39883987
}
39893988

3989+
/* can only be used if rule is no longer visible to dumps */
39903990
static void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule)
39913991
{
3992+
lockdep_commit_lock_is_held(ctx->net);
3993+
39923994
nft_rule_expr_deactivate(ctx, rule, NFT_TRANS_RELEASE);
39933995
nf_tables_rule_destroy(ctx, rule);
39943996
}
@@ -5757,6 +5759,8 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
57575759
struct nft_set_binding *binding,
57585760
enum nft_trans_phase phase)
57595761
{
5762+
lockdep_commit_lock_is_held(ctx->net);
5763+
57605764
switch (phase) {
57615765
case NFT_TRANS_PREPARE_ERROR:
57625766
nft_set_trans_unbind(ctx, set);
@@ -11695,19 +11699,6 @@ static void __nft_release_basechain_now(struct nft_ctx *ctx)
1169511699
nf_tables_chain_destroy(ctx->chain);
1169611700
}
1169711701

11698-
static void nft_release_basechain_rcu(struct rcu_head *head)
11699-
{
11700-
struct nft_chain *chain = container_of(head, struct nft_chain, rcu_head);
11701-
struct nft_ctx ctx = {
11702-
.family = chain->table->family,
11703-
.chain = chain,
11704-
.net = read_pnet(&chain->table->net),
11705-
};
11706-
11707-
__nft_release_basechain_now(&ctx);
11708-
put_net(ctx.net);
11709-
}
11710-
1171111702
int __nft_release_basechain(struct nft_ctx *ctx)
1171211703
{
1171311704
struct nft_rule *rule;
@@ -11722,11 +11713,18 @@ int __nft_release_basechain(struct nft_ctx *ctx)
1172211713
nft_chain_del(ctx->chain);
1172311714
nft_use_dec(&ctx->table->use);
1172411715

11725-
if (maybe_get_net(ctx->net))
11726-
call_rcu(&ctx->chain->rcu_head, nft_release_basechain_rcu);
11727-
else
11716+
if (!maybe_get_net(ctx->net)) {
1172811717
__nft_release_basechain_now(ctx);
11718+
return 0;
11719+
}
11720+
11721+
/* wait for ruleset dumps to complete. Owning chain is no longer in
11722+
* lists, so new dumps can't find any of these rules anymore.
11723+
*/
11724+
synchronize_rcu();
1172911725

11726+
__nft_release_basechain_now(ctx);
11727+
put_net(ctx->net);
1173011728
return 0;
1173111729
}
1173211730
EXPORT_SYMBOL_GPL(__nft_release_basechain);

‎net/netfilter/xt_IDLETIMER.c‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -407,21 +407,23 @@ static void idletimer_tg_destroy(const struct xt_tgdtor_param *par)
407407

408408
mutex_lock(&list_mutex);
409409

410-
if (--info->timer->refcnt == 0) {
411-
pr_debug("deleting timer %s\n", info->label);
412-
413-
list_del(&info->timer->entry);
414-
timer_shutdown_sync(&info->timer->timer);
415-
cancel_work_sync(&info->timer->work);
416-
sysfs_remove_file(idletimer_tg_kobj, &info->timer->attr.attr);
417-
kfree(info->timer->attr.attr.name);
418-
kfree(info->timer);
419-
} else {
410+
if (--info->timer->refcnt > 0) {
420411
pr_debug("decreased refcnt of timer %s to %u\n",
421412
info->label, info->timer->refcnt);
413+
mutex_unlock(&list_mutex);
414+
return;
422415
}
423416

417+
pr_debug("deleting timer %s\n", info->label);
418+
419+
list_del(&info->timer->entry);
424420
mutex_unlock(&list_mutex);
421+
422+
timer_shutdown_sync(&info->timer->timer);
423+
cancel_work_sync(&info->timer->work);
424+
sysfs_remove_file(idletimer_tg_kobj, &info->timer->attr.attr);
425+
kfree(info->timer->attr.attr.name);
426+
kfree(info->timer);
425427
}
426428

427429
static void idletimer_tg_destroy_v1(const struct xt_tgdtor_param *par)
@@ -432,25 +434,27 @@ static void idletimer_tg_destroy_v1(const struct xt_tgdtor_param *par)
432434

433435
mutex_lock(&list_mutex);
434436

435-
if (--info->timer->refcnt == 0) {
436-
pr_debug("deleting timer %s\n", info->label);
437-
438-
list_del(&info->timer->entry);
439-
if (info->timer->timer_type & XT_IDLETIMER_ALARM) {
440-
alarm_cancel(&info->timer->alarm);
441-
} else {
442-
timer_shutdown_sync(&info->timer->timer);
443-
}
444-
cancel_work_sync(&info->timer->work);
445-
sysfs_remove_file(idletimer_tg_kobj, &info->timer->attr.attr);
446-
kfree(info->timer->attr.attr.name);
447-
kfree(info->timer);
448-
} else {
437+
if (--info->timer->refcnt > 0) {
449438
pr_debug("decreased refcnt of timer %s to %u\n",
450439
info->label, info->timer->refcnt);
440+
mutex_unlock(&list_mutex);
441+
return;
451442
}
452443

444+
pr_debug("deleting timer %s\n", info->label);
445+
446+
list_del(&info->timer->entry);
453447
mutex_unlock(&list_mutex);
448+
449+
if (info->timer->timer_type & XT_IDLETIMER_ALARM) {
450+
alarm_cancel(&info->timer->alarm);
451+
} else {
452+
timer_shutdown_sync(&info->timer->timer);
453+
}
454+
cancel_work_sync(&info->timer->work);
455+
sysfs_remove_file(idletimer_tg_kobj, &info->timer->attr.attr);
456+
kfree(info->timer->attr.attr.name);
457+
kfree(info->timer);
454458
}
455459

456460

‎tools/testing/selftests/net/netfilter/rpath.sh‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,20 @@ ip -net "$ns2" a a 192.168.42.1/24 dev d0
6161
ip -net "$ns1" a a fec0:42::2/64 dev v0 nodad
6262
ip -net "$ns2" a a fec0:42::1/64 dev d0 nodad
6363

64+
# avoid neighbor lookups and enable martian IPv6 pings
65+
ns2_hwaddr=$(ip -net "$ns2" link show dev v0 | \
66+
sed -n 's, *link/ether \([^ ]*\) .*,\1,p')
67+
ns1_hwaddr=$(ip -net "$ns1" link show dev v0 | \
68+
sed -n 's, *link/ether \([^ ]*\) .*,\1,p')
69+
ip -net "$ns1" neigh add fec0:42::1 lladdr "$ns2_hwaddr" nud permanent dev v0
70+
ip -net "$ns1" neigh add fec0:23::1 lladdr "$ns2_hwaddr" nud permanent dev v0
71+
ip -net "$ns2" neigh add fec0:42::2 lladdr "$ns1_hwaddr" nud permanent dev d0
72+
ip -net "$ns2" neigh add fec0:23::2 lladdr "$ns1_hwaddr" nud permanent dev v0
73+
6474
# firewall matches to test
6575
[ -n "$iptables" ] && {
6676
common='-t raw -A PREROUTING -s 192.168.0.0/16'
77+
common+=' -p icmp --icmp-type echo-request'
6778
if ! ip netns exec "$ns2" "$iptables" $common -m rpfilter;then
6879
echo "Cannot add rpfilter rule"
6980
exit $ksft_skip
@@ -72,6 +83,7 @@ ip -net "$ns2" a a fec0:42::1/64 dev d0 nodad
7283
}
7384
[ -n "$ip6tables" ] && {
7485
common='-t raw -A PREROUTING -s fec0::/16'
86+
common+=' -p icmpv6 --icmpv6-type echo-request'
7587
if ! ip netns exec "$ns2" "$ip6tables" $common -m rpfilter;then
7688
echo "Cannot add rpfilter rule"
7789
exit $ksft_skip
@@ -82,8 +94,10 @@ ip -net "$ns2" a a fec0:42::1/64 dev d0 nodad
8294
table inet t {
8395
chain c {
8496
type filter hook prerouting priority raw;
85-
ip saddr 192.168.0.0/16 fib saddr . iif oif exists counter
86-
ip6 saddr fec0::/16 fib saddr . iif oif exists counter
97+
ip saddr 192.168.0.0/16 icmp type echo-request \
98+
fib saddr . iif oif exists counter
99+
ip6 saddr fec0::/16 icmpv6 type echo-request \
100+
fib saddr . iif oif exists counter
87101
}
88102
}
89103
EOF

0 commit comments

Comments
 (0)