| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z-QcD5BXD5mY3BA_@mini-arch>
Date: Wed, 26 Mar 2025 08:23:59 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Cosmin Ratiu <cratiu@...dia.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"sdf@...ichev.me" <sdf@...ichev.me>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>
Subject: Re: [PATCH net-next 2/9] net: hold instance lock during
NETDEV_REGISTER/UP/UNREGISTER
On 03/26, Cosmin Ratiu wrote:
> On Tue, 2025-03-25 at 14:30 -0700, Stanislav Fomichev wrote:
> > @@ -2072,8 +2087,8 @@ static void
> > __move_netdevice_notifier_net(struct net *src_net,
> > struct net *dst_net,
> > struct notifier_block *nb)
> > {
> > - __unregister_netdevice_notifier_net(src_net, nb);
> > - __register_netdevice_notifier_net(dst_net, nb, true);
> > + __unregister_netdevice_notifier_net(src_net, nb, false);
> > + __register_netdevice_notifier_net(dst_net, nb, true, false);
> > }
>
> I tested with your (and the rest of Jakub's) patches.
> The problem with this approach is that when a netdev's net is changed,
> its lock will be acquired, but the notifiers for ALL netdevs in the old
> and the new namespace will be called, which will result in correct
> behavior for that device and lockdep_assert_held failure for all
> others.
Perfect, thanks for testing! At least we don't deadlock anymore, that's
progress :-) So looks like we need to do something like the following
below, maybe you can give it a run on your side? Since we don't
have any locking hierarchy (yet), we should be able to lock all
other netdevs besides the one that's already locked at netlink level.
diff --git a/net/core/dev.c b/net/core/dev.c
index afee19245401..125af0fc25d3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1895,32 +1895,32 @@ static int call_netdevice_register_notifiers(struct notifier_block *nb,
static void call_netdevice_unregister_notifiers(struct notifier_block *nb,
struct net_device *dev,
- bool lock)
+ struct net_device *locked)
{
if (dev->flags & IFF_UP) {
call_netdevice_notifier(nb, NETDEV_GOING_DOWN,
dev);
call_netdevice_notifier(nb, NETDEV_DOWN, dev);
}
- if (lock)
+ if (dev != locked)
netdev_lock_ops(dev);
call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
- if (lock)
+ if (dev != locked)
netdev_unlock_ops(dev);
}
static int call_netdevice_register_net_notifiers(struct notifier_block *nb,
struct net *net,
- bool lock)
+ struct net_device *locked)
{
struct net_device *dev;
int err;
for_each_netdev(net, dev) {
- if (lock)
+ if (locked != dev)
netdev_lock_ops(dev);
err = call_netdevice_register_notifiers(nb, dev);
- if (lock)
+ if (locked != dev)
netdev_unlock_ops(dev);
if (err)
goto rollback;
@@ -1929,18 +1929,18 @@ static int call_netdevice_register_net_notifiers(struct notifier_block *nb,
rollback:
for_each_netdev_continue_reverse(net, dev)
- call_netdevice_unregister_notifiers(nb, dev, lock);
+ call_netdevice_unregister_notifiers(nb, dev, locked);
return err;
}
static void call_netdevice_unregister_net_notifiers(struct notifier_block *nb,
struct net *net,
- bool lock)
+ struct net_device *locked)
{
struct net_device *dev;
for_each_netdev(net, dev)
- call_netdevice_unregister_notifiers(nb, dev, lock);
+ call_netdevice_unregister_notifiers(nb, dev, locked);
}
static int dev_boot_phase = 1;
@@ -1977,7 +1977,7 @@ int register_netdevice_notifier(struct notifier_block *nb)
goto unlock;
for_each_net(net) {
__rtnl_net_lock(net);
- err = call_netdevice_register_net_notifiers(nb, net, true);
+ err = call_netdevice_register_net_notifiers(nb, net, NULL);
__rtnl_net_unlock(net);
if (err)
goto rollback;
@@ -1991,7 +1991,7 @@ int register_netdevice_notifier(struct notifier_block *nb)
rollback:
for_each_net_continue_reverse(net) {
__rtnl_net_lock(net);
- call_netdevice_unregister_net_notifiers(nb, net, true);
+ call_netdevice_unregister_net_notifiers(nb, net, NULL);
__rtnl_net_unlock(net);
}
@@ -2028,7 +2028,7 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
for_each_net(net) {
__rtnl_net_lock(net);
- call_netdevice_unregister_net_notifiers(nb, net, true);
+ call_netdevice_unregister_net_notifiers(nb, net, NULL);
__rtnl_net_unlock(net);
}
@@ -2042,7 +2042,7 @@ EXPORT_SYMBOL(unregister_netdevice_notifier);
static int __register_netdevice_notifier_net(struct net *net,
struct notifier_block *nb,
bool ignore_call_fail,
- bool lock)
+ struct net_device *locked)
{
int err;
@@ -2052,7 +2052,7 @@ static int __register_netdevice_notifier_net(struct net *net,
if (dev_boot_phase)
return 0;
- err = call_netdevice_register_net_notifiers(nb, net, lock);
+ err = call_netdevice_register_net_notifiers(nb, net, locked);
if (err && !ignore_call_fail)
goto chain_unregister;
@@ -2065,7 +2065,7 @@ static int __register_netdevice_notifier_net(struct net *net,
static int __unregister_netdevice_notifier_net(struct net *net,
struct notifier_block *nb,
- bool lock)
+ struct net_device *locked)
{
int err;
@@ -2073,7 +2073,7 @@ static int __unregister_netdevice_notifier_net(struct net *net,
if (err)
return err;
- call_netdevice_unregister_net_notifiers(nb, net, lock);
+ call_netdevice_unregister_net_notifiers(nb, net, locked);
return 0;
}
@@ -2097,7 +2097,7 @@ int register_netdevice_notifier_net(struct net *net, struct notifier_block *nb)
int err;
rtnl_net_lock(net);
- err = __register_netdevice_notifier_net(net, nb, false, true);
+ err = __register_netdevice_notifier_net(net, nb, false, NULL);
rtnl_net_unlock(net);
return err;
@@ -2126,7 +2126,7 @@ int unregister_netdevice_notifier_net(struct net *net,
int err;
rtnl_net_lock(net);
- err = __unregister_netdevice_notifier_net(net, nb, true);
+ err = __unregister_netdevice_notifier_net(net, nb, NULL);
rtnl_net_unlock(net);
return err;
@@ -2135,10 +2135,11 @@ EXPORT_SYMBOL(unregister_netdevice_notifier_net);
static void __move_netdevice_notifier_net(struct net *src_net,
struct net *dst_net,
- struct notifier_block *nb)
+ struct notifier_block *nb,
+ struct net_device *locked)
{
- __unregister_netdevice_notifier_net(src_net, nb, false);
- __register_netdevice_notifier_net(dst_net, nb, true, false);
+ __unregister_netdevice_notifier_net(src_net, nb, locked);
+ __register_netdevice_notifier_net(dst_net, nb, true, locked);
}
static void rtnl_net_dev_lock(struct net_device *dev)
@@ -2184,7 +2185,7 @@ int register_netdevice_notifier_dev_net(struct net_device *dev,
int err;
rtnl_net_dev_lock(dev);
- err = __register_netdevice_notifier_net(dev_net(dev), nb, false, true);
+ err = __register_netdevice_notifier_net(dev_net(dev), nb, false, NULL);
if (!err) {
nn->nb = nb;
list_add(&nn->list, &dev->net_notifier_list);
@@ -2203,7 +2204,7 @@ int unregister_netdevice_notifier_dev_net(struct net_device *dev,
rtnl_net_dev_lock(dev);
list_del(&nn->list);
- err = __unregister_netdevice_notifier_net(dev_net(dev), nb, true);
+ err = __unregister_netdevice_notifier_net(dev_net(dev), nb, NULL);
rtnl_net_dev_unlock(dev);
return err;
@@ -2216,7 +2217,7 @@ static void move_netdevice_notifiers_dev_net(struct net_device *dev,
struct netdev_net_notifier *nn;
list_for_each_entry(nn, &dev->net_notifier_list, list)
- __move_netdevice_notifier_net(dev_net(dev), net, nn->nb);
+ __move_netdevice_notifier_net(dev_net(dev), net, nn->nb, dev);
}
/**
Powered by blists - more mailing lists