[PATCH 0/3] Avoid calling WARN_ON() on allocation failure in cfg802154_switch_netns()

[PATCH 0/3] Avoid calling WARN_ON() on allocation failure in cfg802154_switch_netns()

[Ivan Abramov]

This series was inspired by Syzkaller report on warning in
cfg802154_switch_netns().

WARNING: CPU: 0 PID: 5837 at net/ieee802154/core.c:258 cfg802154_switch_netns+0x3c7/0x3d0 net/ieee802154/core.c:258
Modules linked in:
CPU: 0 UID: 0 PID: 5837 Comm: syz-executor125 Not tainted 6.13.0-rc6-syzkaller-00918-g7b24f164cf00 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:cfg802154_switch_netns+0x3c7/0x3d0 net/ieee802154/core.c:258
Call Trace:
 <TASK>
 nl802154_wpan_phy_netns+0x13d/0x210 net/ieee802154/nl802154.c:1292
 genl_family_rcv_msg_doit net/netlink/genetlink.c:1115 [inline]
 genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
 genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1210
 netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2543
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
 netlink_unicast_kernel net/netlink/af_netlink.c:1322 [inline]
 netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1348
 netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1892
 sock_sendmsg_nosec net/socket.c:711 [inline]
 __sock_sendmsg+0x221/0x270 net/socket.c:726
 ____sys_sendmsg+0x52a/0x7e0 net/socket.c:2594
 ___sys_sendmsg net/socket.c:2648 [inline]
 __sys_sendmsg+0x269/0x350 net/socket.c:2680
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

This warning is caused by Syzkaller's fault injection, which causes
kstrdup() in device_rename() to fail, so device_rename() returns -ENOMEM.

Since practically such failure is not possible, avoid it, additionally
fixing similar pointless allocation-related warnings.

Ivan Abramov (3):
  ieee802154: Restore initial state on failed device_rename() in
    cfg802154_switch_netns()
  ieee802154: Avoid calling WARN_ON() on -ENOMEM in
    cfg802154_switch_netns()
  ieee802154: Remove WARN_ON() in cfg802154_pernet_exit()

 net/ieee802154/core.c | 51 ++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

-- 
2.39.5

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

[Ivan Abramov]

Currently, the return value of device_rename() is not acted upon.

To avoid an inconsistent state in case of failure, roll back the changes
made before the device_rename() call.

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 | 45 ++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 88adb04e4072..ddde594513a0 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -233,31 +233,36 @@ 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;
-		}
-
-		return err;
-	}
-
-	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);
 
+	if (err)
+		goto errout;
+
+	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 2/3] ieee802154: Avoid calling WARN_ON() on -ENOMEM in cfg802154_switch_netns()

[Ivan Abramov]

It's pointless to call WARN_ON() in case of an allocation failure in
dev_change_net_namespace() and 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: 66e5c2672cd1 ("ieee802154: add netns support")
Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>
---
 net/ieee802154/core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index ddde594513a0..8c47957f5df7 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -228,8 +228,10 @@ int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
 			continue;
 		wpan_dev->netdev->netns_local = false;
 		err = dev_change_net_namespace(wpan_dev->netdev, net, "wpan%d");
-		if (err)
+		if (err) {
+			WARN_ON(err && err != -ENOMEM);
 			break;
+		}
 		wpan_dev->netdev->netns_local = true;
 	}
 
@@ -237,7 +239,7 @@ int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
 		goto errout;
 
 	err = device_rename(&rdev->wpan_phy.dev, dev_name(&rdev->wpan_phy.dev));
-	WARN_ON(err);
+	WARN_ON(err && err != -ENOMEM);
 
 	if (err)
 		goto errout;
@@ -258,7 +260,7 @@ int cfg802154_switch_netns(struct cfg802154_registered_device *rdev,
 		wpan_dev->netdev->netns_local = false;
 		err = dev_change_net_namespace(wpan_dev->netdev, net,
 					       "wpan%d");
-		WARN_ON(err);
+		WARN_ON(err && err != -ENOMEM);
 		wpan_dev->netdev->netns_local = true;
 	}
 
-- 
2.39.5

[PATCH 3/3] ieee802154: Remove WARN_ON() in cfg802154_pernet_exit()

[Ivan Abramov]

There's no need to call WARN_ON() in cfg802154_pernet_exit(), since
every point of failure in cfg802154_switch_netns() is covered with
WARN_ON(), so remove 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 8c47957f5df7..274112ff2955 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -358,7 +358,7 @@ static void __net_exit cfg802154_pernet_exit(struct net *net)
 	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));
+			cfg802154_switch_netns(rdev, &init_net);
 	}
 	rtnl_unlock();
 }
-- 
2.39.5

Re: [PATCH 2/3] ieee802154: Avoid calling WARN_ON() on -ENOMEM in cfg802154_switch_netns()

[Kuniyuki Iwashima]

From: Ivan Abramov <i.abramov@mt-integration.ru>
Date: Fri, 28 Mar 2025 04:04:26 +0300
> It's pointless to call WARN_ON() in case of an allocation failure in
> dev_change_net_namespace() and 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: 66e5c2672cd1 ("ieee802154: add netns support")
> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>

I suggested using net_warn_ratelimited() so this tag is not needed.
The patch itself looks good to me:

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>

Reported-by: syzbot+e0bd4e4815a910c0daa8@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/000000000000f4a1b7061f9421de@google.com/#t

Re: [PATCH 0/3] Avoid calling WARN_ON() on allocation failure in cfg802154_switch_netns()

[Kuniyuki Iwashima]

From: Ivan Abramov <i.abramov@mt-integration.ru>
Date: Fri, 28 Mar 2025 04:04:24 +0300
> This series was inspired by Syzkaller report on warning in
> cfg802154_switch_netns().
> 
> WARNING: CPU: 0 PID: 5837 at net/ieee802154/core.c:258 cfg802154_switch_netns+0x3c7/0x3d0 net/ieee802154/core.c:258
> Modules linked in:
> CPU: 0 UID: 0 PID: 5837 Comm: syz-executor125 Not tainted 6.13.0-rc6-syzkaller-00918-g7b24f164cf00 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> RIP: 0010:cfg802154_switch_netns+0x3c7/0x3d0 net/ieee802154/core.c:258
> Call Trace:
>  <TASK>
>  nl802154_wpan_phy_netns+0x13d/0x210 net/ieee802154/nl802154.c:1292
>  genl_family_rcv_msg_doit net/netlink/genetlink.c:1115 [inline]
>  genl_family_rcv_msg net/netlink/genetlink.c:1195 [inline]
>  genl_rcv_msg+0xb14/0xec0 net/netlink/genetlink.c:1210
>  netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2543
>  genl_rcv+0x28/0x40 net/netlink/genetlink.c:1219
>  netlink_unicast_kernel net/netlink/af_netlink.c:1322 [inline]
>  netlink_unicast+0x7f6/0x990 net/netlink/af_netlink.c:1348
>  netlink_sendmsg+0x8e4/0xcb0 net/netlink/af_netlink.c:1892
>  sock_sendmsg_nosec net/socket.c:711 [inline]
>  __sock_sendmsg+0x221/0x270 net/socket.c:726
>  ____sys_sendmsg+0x52a/0x7e0 net/socket.c:2594
>  ___sys_sendmsg net/socket.c:2648 [inline]
>  __sys_sendmsg+0x269/0x350 net/socket.c:2680
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> This warning is caused by Syzkaller's fault injection, which causes
> kstrdup() in device_rename() to fail, so device_rename() returns -ENOMEM.
> 
> Since practically such failure is not possible, avoid it, additionally
> fixing similar pointless allocation-related warnings.
> 
> Ivan Abramov (3):
>   ieee802154: Restore initial state on failed device_rename() in
>     cfg802154_switch_netns()
>   ieee802154: Avoid calling WARN_ON() on -ENOMEM in
>     cfg802154_switch_netns()
>   ieee802154: Remove WARN_ON() in cfg802154_pernet_exit()

While at it, apply the same change to cfg80211_switch_netns() and
cfg80211_pernet_exit().

Thanks!


> 
>  net/ieee802154/core.c | 51 ++++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)
> 
> -- 

Re: [PATCH 0/3] Avoid calling WARN_ON() on allocation failure in cfg802154_switch_netns()

[Miquel Raynal]

Hello Ivan,

On 28/03/2025 at 04:04:24 +03, Ivan Abramov <i.abramov@mt-integration.ru> wrote:

> This series was inspired by Syzkaller report on warning in
> cfg802154_switch_netns().

Thanks for the series, lgtm.

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Miquèl

Re: [PATCH 2/3] ieee802154: Avoid calling WARN_ON() on -ENOMEM in cfg802154_switch_netns()

[Ivan Abramov]

On Thu, 27 Mar 2025 19:30:02 -0700, Kuniyuki Iwashima wrote:
> From: Ivan Abramov <i.abramov@mt-integration.ru>
> Date: Fri, 28 Mar 2025 04:04:26 +0300
>> It's pointless to call WARN_ON() in case of an allocation failure in
>> dev_change_net_namespace() and 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: 66e5c2672cd1 ("ieee802154: add netns support")
>> Suggested-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
> I suggested using net_warn_ratelimited() so this tag is not needed.
> The patch itself looks good to me:

Should I send v2 series with fixed tags?

Thank you for reviewing the series!


> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
>
>> Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru>
>
> Reported-by: syzbot+e0bd4e4815a910c0daa8@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/000000000000f4a1b7061f9421de@google.com/#t

-- 
Ivan Abramov <i.abramov@mt-integration.ru>