[PATCH net 0/4] Avoid using WARN_ON() on allocation failure in device_rename()

[PATCH net 0/4] Avoid using WARN_ON() on allocation failure in device_rename()

[Ivan Abramov]

This patch series is based on
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ and is
intended for the generic netdev maintainers, as it affects multiple
networking subsystems.

There are a couple of Syzkaller reports about WARN_ON() being triggered
by failed device_rename().

They are triggered by fuzzer's fault injection and subsequent allocation
failure in kstrdup(). Failure of kstrdup() in device_rename() should not
lead to WARN_ON(), so means to avoid it are introduced in this series.

If it is possible to reverse the changes done prior to failed
device_rename(), do that. Otherwise ignore -ENOMEM return code in
WARN_ON().

Ivan Abramov (4):
  ieee802154: Restore initial state on failed device_rename() in
    cfg802154_switch_netns()
  ieee802154: Avoid calling WARN_ON() on -ENOMEM in
    cfg802154_pernet_exit()
  cfg80211: Avoid calling WARN_ON() on -ENOMEM in
    cfg80211_switch_netns()
  net: Avoid calling WARN_ON() on -ENOMEM in
    __dev_change_net_namespace()

 net/core/dev.c        |  2 +-
 net/ieee802154/core.c | 51 ++++++++++++++++++++++++-------------------
 net/wireless/core.c   |  2 +-
 3 files changed, 31 insertions(+), 24 deletions(-)

-- 
2.39.5

[PATCH net 1/4] ieee802154: Restore initial state on failed device_rename() in cfg802154_switch_netns()

[Ivan Abramov]

Currently, the return value of device_rename() is not checked or acted
upon. There is also a pointless WARN_ON() call in case of an allocation
failure, since it only leads to useless splats caused by deliberate fault
injections.

Since it's possible to roll back the changes made before the
device_rename() call in case of failure, do it.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 66e5c2672cd1 ("ieee802154: add netns support")
Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>
---
 net/ieee802154/core.c | 44 +++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 88adb04e4072..f9865eb2c7cf 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -233,31 +233,35 @@ int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
 		wpan_dev->netdev->netns_local = true;
 	}
 
-	if (err) {
-		/* failed -- clean up to old netns */
-		net = wpan_phy_net(&rdev->wpan_phy);
-
-		list_for_each_entry_continue_reverse(wpan_dev,
-						     &rdev->wpan_dev_list,
-						     list) {
-			if (!wpan_dev->netdev)
-				continue;
-			wpan_dev->netdev->netns_local = false;
-			err = dev_change_net_namespace(wpan_dev->netdev, net,
-						       "wpan%d");
-			WARN_ON(err);
-			wpan_dev->netdev->netns_local = true;
-		}
+	if (err)
+		goto errout;
 
-		return err;
-	}
+	err = device_rename(&rdev->wpan_phy.dev, dev_name(&rdev->wpan_phy.dev));
 
-	wpan_phy_net_set(&rdev->wpan_phy, net);
+	if (err)
+		goto errout;
 
-	err = device_rename(&rdev->wpan_phy.dev, dev_name(&rdev->wpan_phy.dev));
-	WARN_ON(err);
+	wpan_phy_net_set(&rdev->wpan_phy, net);
 
 	return 0;
+
+errout:
+	/* failed -- clean up to old netns */
+	net = wpan_phy_net(&rdev->wpan_phy);
+
+	list_for_each_entry_continue_reverse(wpan_dev,
+					     &rdev->wpan_dev_list,
+					     list) {
+		if (!wpan_dev->netdev)
+			continue;
+		wpan_dev->netdev->netns_local = false;
+		err = dev_change_net_namespace(wpan_dev->netdev, net,
+					       "wpan%d");
+		WARN_ON(err);
+		wpan_dev->netdev->netns_local = true;
+	}
+
+	return err;
 }
 
 void cfg802154_dev_free(struct cfg802154_registered_device *rdev)
-- 
2.39.5

[PATCH net 2/4] ieee802154: Avoid calling WARN_ON() on -ENOMEM in cfg802154_pernet_exit()

[Ivan Abramov]

It's pointless to call WARN_ON() in case of an allocation failure in
device_rename(), since it only leads to useless splats caused by deliberate
fault injections, so avoid it.

Fixes: 66e5c2672cd1 ("ieee802154: add netns support")
Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>
---
 net/ieee802154/core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index f9865eb2c7cf..77760ed4e528 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -351,11 +351,14 @@ static struct notifier_block cfg802154_netdev_notifier = {
 static void __net_exit cfg802154_pernet_exit(struct net *net)
 {
 	struct cfg802154_registered_device *rdev;
+	int err;
 
 	rtnl_lock();
 	list_for_each_entry(rdev, &cfg802154_rdev_list, list) {
-		if (net_eq(wpan_phy_net(&rdev->wpan_phy), net))
-			WARN_ON(cfg802154_switch_netns(rdev, &init_net));
+		if (net_eq(wpan_phy_net(&rdev->wpan_phy), net)) {
+			err = cfg802154_switch_netns(rdev, &init_net);
+			WARN_ON(err && err != -ENOMEM);
+		}
 	}
 	rtnl_unlock();
 }
-- 
2.39.5

[PATCH net 3/4] cfg80211: Avoid calling WARN_ON() on -ENOMEM in cfg80211_switch_netns()

[Ivan Abramov]

It's pointless to call WARN_ON() in case of an allocation failure in
device_rename(), since it only leads to useless splats caused by deliberate
fault injections, so avoid it.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 04600794958f ("cfg80211: support sysfs namespaces")
Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>
---
 net/wireless/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 828e29872633..7c0ca2fd3b45 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -201,7 +201,7 @@ int cfg80211_switch_netns(struct cfg80211_registered_device *rdev,
 	wiphy_net_set(&rdev->wiphy, net);
 
 	err = device_rename(&rdev->wiphy.dev, dev_name(&rdev->wiphy.dev));
-	WARN_ON(err);
+	WARN_ON(err && err != -ENOMEM);
 
 	nl80211_notify_wiphy(rdev, NL80211_CMD_NEW_WIPHY);
 
-- 
2.39.5

Re: [PATCH net 0/4] Avoid using WARN_ON() on allocation failure in device_rename()

[Jakub Kicinski]

On Tue, 25 Mar 2025 17:17:19 +0300 Ivan Abramov wrote:
> This patch series is based on
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ and is
> intended for the generic netdev maintainers, as it affects multiple
> networking subsystems.

But there is no dependency between the patches, AFAICT.
Please send them individually to the respective maintainers.
-- 
pw-bot: cr

Re: [PATCH net 1/4] ieee802154: Restore initial state on failed device_rename() in cfg802154_switch_netns()

[Kuniyuki Iwashima]

From: Ivan Abramov <i.abramov@mt-integration.ru>
Date: Tue, 25 Mar 2025 17:17:20 +0300
> Currently, the return value of device_rename() is not checked or acted
> upon. There is also a pointless WARN_ON() call in case of an allocation
> failure, since it only leads to useless splats caused by deliberate fault
> injections.
> 
> Since it's possible to roll back the changes made before the
> device_rename() call in case of failure, do it.
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: 66e5c2672cd1 ("ieee802154: add netns support")
> Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>
> ---
>  net/ieee802154/core.c | 44 +++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
> index 88adb04e4072..f9865eb2c7cf 100644
> --- a/net/ieee802154/core.c
> +++ b/net/ieee802154/core.c
> @@ -233,31 +233,35 @@ int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
>  		wpan_dev->netdev->netns_local = true;
>  	}
>  
> -	if (err) {
> -		/* failed -- clean up to old netns */
> -		net = wpan_phy_net(&rdev->wpan_phy);
> -
> -		list_for_each_entry_continue_reverse(wpan_dev,
> -						     &rdev->wpan_dev_list,
> -						     list) {
> -			if (!wpan_dev->netdev)
> -				continue;
> -			wpan_dev->netdev->netns_local = false;
> -			err = dev_change_net_namespace(wpan_dev->netdev, net,
> -						       "wpan%d");
> -			WARN_ON(err);
> -			wpan_dev->netdev->netns_local = true;
> -		}
> +	if (err)
> +		goto errout;
>  
> -		return err;
> -	}
> +	err = device_rename(&rdev->wpan_phy.dev, dev_name(&rdev->wpan_phy.dev));
>  
> -	wpan_phy_net_set(&rdev->wpan_phy, net);
> +	if (err)
> +		goto errout;
>  
> -	err = device_rename(&rdev->wpan_phy.dev, dev_name(&rdev->wpan_phy.dev));
> -	WARN_ON(err);
> +	wpan_phy_net_set(&rdev->wpan_phy, net);
>  
>  	return 0;
> +
> +errout:
> +	/* failed -- clean up to old netns */
> +	net = wpan_phy_net(&rdev->wpan_phy);
> +
> +	list_for_each_entry_continue_reverse(wpan_dev,
> +					     &rdev->wpan_dev_list,
> +					     list) {
> +		if (!wpan_dev->netdev)
> +			continue;
> +		wpan_dev->netdev->netns_local = false;
> +		err = dev_change_net_namespace(wpan_dev->netdev, net,
> +					       "wpan%d");
> +		WARN_ON(err);

It's still possible to trigger this with -ENOMEM.

For example, see bitmap_zalloc() in __dev_alloc_name().

Perhaps simply use pr_warn() or net_warn_ratelimited() as do_setlink().

I guess the stack trace from here is not so interesting as it doens't
show where it actually failed.


> +		wpan_dev->netdev->netns_local = true;
> +	}
> +
> +	return err;
>  }
>  
>  void cfg802154_dev_free(struct cfg802154_registered_device *rdev)
> -- 
> 2.39.5