| Message ID | 20250403113519.992462-1-i.abramov@mt-integration.ru (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace() | expand |
On 04/03, Ivan Abramov wrote: > 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. What if this happens in a non-fault injection environment? Suppose the user shows up and says that he's having an issue with device changing its name after netns change. There will be no way to diagnose it, right?
On Thu, 3 Apr 2025 11:05:31 -0700, Stanislav Fomichev wrote: > On 04/03, Ivan Abramov wrote: >> 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. > What if this happens in a non-fault injection environment? Suppose > the user shows up and says that he's having an issue with device > changing its name after netns change. There will be no way to diagnose > it, right? Failure to allocate a few hundred bytes in kstrdup doesn't seem practically possible and happens only in fault injection scenarios. Other types of failures in device_rename will still trigger WARN_ON.
On Fri, Apr 4, 2025 at 9:29 AM Ivan Abramov <i.abramov@mt-integration.ru> wrote: > > On Thu, 3 Apr 2025 11:05:31 -0700, Stanislav Fomichev wrote: > > On 04/03, Ivan Abramov wrote: > >> 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. > > > What if this happens in a non-fault injection environment? Suppose > > the user shows up and says that he's having an issue with device > > changing its name after netns change. There will be no way to diagnose > > it, right? > > Failure to allocate a few hundred bytes in kstrdup doesn't seem > practically possible and happens only in fault injection scenarios. Other > types of failures in device_rename will still trigger WARN_ON. If you want to fix this, fix it properly. Do not paper around the issue by silencing a warning.
diff --git a/net/core/dev.c b/net/core/dev.c index be17e0660144..001c362a4c1d 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -12196,7 +12196,7 @@ int netif_change_net_namespace(struct net_device *dev, struct net *net, dev_set_uevent_suppress(&dev->dev, 1); err = device_rename(&dev->dev, dev->name); dev_set_uevent_suppress(&dev->dev, 0); - WARN_ON(err); + WARN_ON(err && err != -ENOMEM); /* Send a netdev-add uevent to the new namespace */ kobject_uevent(&dev->dev.kobj, KOBJ_ADD);
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: 8b41d1887db7 ("[NET]: Fix running without sysfs") Reported-by: syzbot+1df6ffa7a6274ae264db@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/000000000000a45a92061ce6cc7d@google.com/ Link: https://lore.kernel.org/netdev/20250328011302.743860-1-i.abramov@mt-integration.ru/ Signed-off-by: Ivan Abramov <i.abramov@mt-integration.ru> --- v2: Add Reported-by and Closes tags and make sure to commit against latest netdev/net as per Kuniyuki Iwashima's observation. Also add Link tag. net/core/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)