Thread (13 messages) 13 messages, 1 author, 9h ago

Re: [PATCH v1 net-next 07/10] net: fib_rules: Drop RTNL assertions.

From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2026-06-30 19:52:49

Replying to Sashiko review.

On Mon, Jun 29, 2026 at 11:12 AM Kuniyuki Iwashima [off-list ref] wrote:
Now, fib_rule structs are protected by per-fib_rules_ops mutex.

Let's drop ASSERT_RTNL_NET() and rtnl_dereference().

Note that fib_rules_event() iterates over net->rules_ops without
net->rules_mod_lock, but this is fine because all fib_rule users
are built-in and concurrent fib_rules_unregister() does not happen.
---8<---
Is this description accurate?
---8<---

Yes.


---8<---
It appears that unprivileged users can trigger a race here via user
namespaces. If a user creates two network namespaces with a veth pair, moving
one interface to each, and then triggers netns destruction on one namespace
while concurrently deleting the veth interface from the other namespace:
...
---8<---

Sashiko somehow thinks it's possible to remove a netdev and dismantle
another netns, both of which require RTNL, so false-positive.


quoted hunk ↗ jump to hunk
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/core/fib_rules.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 25a3fd997782..5eef5d6ace82 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -387,7 +387,6 @@ static int call_fib_rule_notifiers(struct net *net,
                .rule = rule,
        };

-       ASSERT_RTNL_NET(net);
        lockdep_assert_held(&ops->lock);

        /* Paired with READ_ONCE() in fib_rules_seq() */
@@ -955,7 +954,7 @@ int fib_newrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
                list_for_each_entry(r, &ops->rules_list, list) {
                        if (r->action == FR_ACT_GOTO &&
                            r->target == rule->pref &&
-                           rtnl_dereference(r->ctarget) == NULL) {
+                           !rcu_access_pointer(r->ctarget)) {
                                rcu_assign_pointer(r->ctarget, rule);
                                if (--ops->unresolved_rules == 0)
                                        break;
@@ -1064,7 +1063,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,

        if (rule->action == FR_ACT_GOTO) {
                ops->nr_goto_rules--;
-               if (rtnl_dereference(rule->ctarget) == NULL)
+               if (!rcu_access_pointer(rule->ctarget))
                        ops->unresolved_rules--;
        }
@@ -1082,7 +1081,7 @@ int fib_delrule(struct net *net, struct sk_buff *skb, struct nlmsghdr *nlh,
                if (&n->list == &ops->rules_list || n->pref != rule->pref)
                        n = NULL;
                list_for_each_entry(r, &ops->rules_list, list) {
-                       if (rtnl_dereference(r->ctarget) != rule)
+                       if (rcu_access_pointer(r->ctarget) != rule)
                                continue;
                        rcu_assign_pointer(r->ctarget, n);
                        if (!n)
@@ -1400,8 +1399,6 @@ static int fib_rules_event(struct notifier_block *this, unsigned long event,
        struct net *net = dev_net(dev);
        struct fib_rules_ops *ops;

-       ASSERT_RTNL();
-
        switch (event) {
        case NETDEV_REGISTER:
                list_for_each_entry(ops, &net->rules_ops, list) {
--
2.55.0.rc0.799.gd6f94ed593-goog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help