[PATCH net v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace()

[PATCH net v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace()

[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: 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(-)

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);
-- 
2.39.5

Re: [PATCH net v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace()

[Stanislav Fomichev]

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?

Re: [PATCH net v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace()

[Ivan Abramov]

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.

Re: [PATCH net v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace()

[Eric Dumazet]

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.

Re: [PATCH net v2] net: Avoid calling WARN_ON() on -ENOMEM in netif_change_net_namespace()

[Ivan Abramov]

On Fri, 4 Apr 2025 10:53:35 +0200, Eric Dumazet wrote:
> 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.

As far as I know, WARN_ON call on -ENOMEM is the issue itself, since
it only fires in testing/deliberate scenarios. And this fixes just that,
without touching anything else.

How should proper fix of this look like? I would be glad to work up
some solution that satisfies maintainers' vision, but I don't see other
ways around it without some grand refactoring, which may bring more
problems than solutions.