| Message ID | 20250325141723.499850-2-i.abramov@mt-integration.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Avoid using WARN_ON() on allocation failure in device_rename() | expand |
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
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)
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(-)