mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ipv4: guard inetdev_event() against a NULL from inetdev_init()
@ 2026-09-21 15:55 Yunseong Kim
  2026-09-21 16:09 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Yunseong Kim @ 2026-09-21 15:55 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: David Ahern, Ido Schimmel, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Taehee Yoo,
	Yunseong Kim, stable, syzkaller-bugs

On NETDEV_REGISTER, inetdev_event() only checks the inetdev_init()
return value with IS_ERR():

    in_dev = inetdev_init(dev);
    if (IS_ERR(in_dev))
        return notifier_from_errno(PTR_ERR(in_dev));
    if (dev->flags & IFF_LOOPBACK) {
        IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
    ...

Because IS_ERR(NULL) is false, a NULL return flows straight into
IN_DEV_CONF_SET() -> ipv4_devconf_set(), whose
set_bit(index, in_dev->cnf.state) dereferences NULL.

inetdev_init() is only kept from returning NULL by its trailing

    out:
        return in_dev ?: ERR_PTR(err);

which relies on every failure path that leaves in_dev NULL having also
set a non-zero err. That invariant is fragile and lives in the producer,
while the consumer's IS_ERR()-only check silently depends on it: any
future inetdev_init() failure path that returns NULL (directly, or by
leaving err == 0) becomes a NULL dereference at this call site rather
than a clean error return.

Before commit 20e61da7ffcf ("ipv4: fail early when creating netdev
named all or default") this call site used "if (!in_dev)", which caught
a NULL return; that commit converted inetdev_init() to the ERR_PTR()
convention and switched the check to IS_ERR(), dropping the NULL
handling here.

Decouple the caller from that invariant by using IS_ERR_OR_NULL() and
translating a NULL return to -ENOMEM, so a NULL can no longer be
dereferenced regardless of how inetdev_init() signals failure.

Cc: stable@vger.kernel.org
Cc: syzkaller-bugs@googlegroups.com
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
---

Syzkaller reproducer:
# {Threaded:true Repeat:true RepeatTimes:0 Procs:8 Slowdown:1 Sandbox:none SandboxArg:0 Leak:false NetInjection:false NetDevices:true NetReset:true Cgroups:true BinfmtMisc:true CloseFDs:true KCSAN:false DevlinkP
CI:false NicVF:false USB:false VhciInjection:false Wifi:false IEEE802154:false Sysctl:true Swap:true UseTmpDir:true HandleSegv:true Trace:false CallComments:true LegacyOptions:{Collide:false Fault:false FaultCal
l:0 FaultNth:0}}
r0 = creat(&(0x7f00000000c0)='./file0\x00', 0x26)
ioctl$RNDADDTOENTCNT(r0, 0x40045201, 0x0)
r1 = socket$inet6_udplite(0xa, 0x2, 0x88)
setsockopt$sock_int(r1, 0x1, 0x1d, &(0x7f0000000380), 0x4) (async, rerun: 64)
unshare(0x42020000) (async, rerun: 64)
truncate(&(0x7f0000000040)='./file0\x00', 0x9) (rerun: 64)
ioctl$sock_SIOCBRADDBR(0xffffffffffffffff, 0x89a0, &(0x7f0000000000)='syzkaller0\x00')
socket$inet_udplite(0x2, 0x2, 0x88)
rename(&(0x7f0000000640)='./file0aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\x00', &(0x7f0000000780)='./file1\x00') (async)
ioctl$BTRFS_IOC_BALANCE_PROGRESS(0xffffffffffffffff, 0x84009422, &(0x7f00000004c0)={0x0, 0x0, {0x0, @struct, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @struct}, {}, {0x0, @struct}})

 net/ipv4/devinet.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a90be57c63be..353d7c584463 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1588,8 +1588,9 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
 	if (!in_dev) {
 		if (event == NETDEV_REGISTER) {
 			in_dev = inetdev_init(dev);
-			if (IS_ERR(in_dev))
-				return notifier_from_errno(PTR_ERR(in_dev));
+			if (IS_ERR_OR_NULL(in_dev))
+				return notifier_from_errno(in_dev ?
+						PTR_ERR(in_dev) : -ENOMEM);
 			if (dev->flags & IFF_LOOPBACK) {
 				IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
 				IN_DEV_CONF_SET(in_dev, NOPOLICY, 1);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ipv4: guard inetdev_event() against a NULL from inetdev_init()
  2026-09-21 15:55 [PATCH] ipv4: guard inetdev_event() against a NULL from inetdev_init() Yunseong Kim
@ 2026-09-21 16:09 ` Eric Dumazet
  2026-09-21 18:56   ` Yunseong Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-09-21 16:09 UTC (permalink / raw)
  To: Yunseong Kim
  Cc: netdev, linux-kernel, David Ahern, Ido Schimmel, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Taehee Yoo, stable,
	syzkaller-bugs

On Mon, Sep 21, 2026 at 5:58 PM Yunseong Kim <yunseong.kim@est.tech> wrote:
>
> On NETDEV_REGISTER, inetdev_event() only checks the inetdev_init()
> return value with IS_ERR():
>
>     in_dev = inetdev_init(dev);
>     if (IS_ERR(in_dev))
>         return notifier_from_errno(PTR_ERR(in_dev));
>     if (dev->flags & IFF_LOOPBACK) {
>         IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
>     ...
>
> Because IS_ERR(NULL) is false, a NULL return flows straight into
> IN_DEV_CONF_SET() -> ipv4_devconf_set(), whose
> set_bit(index, in_dev->cnf.state) dereferences NULL.
>
> inetdev_init() is only kept from returning NULL by its trailing
>
>     out:
>         return in_dev ?: ERR_PTR(err);
>
> which relies on every failure path that leaves in_dev NULL having also
> set a non-zero err. That invariant is fragile and lives in the producer,
> while the consumer's IS_ERR()-only check silently depends on it: any
> future inetdev_init() failure path that returns NULL (directly, or by
> leaving err == 0) becomes a NULL dereference at this call site rather
> than a clean error return.
>
> Before commit 20e61da7ffcf ("ipv4: fail early when creating netdev
> named all or default") this call site used "if (!in_dev)", which caught
> a NULL return; that commit converted inetdev_init() to the ERR_PTR()
> convention and switched the check to IS_ERR(), dropping the NULL
> handling here.
>
> Decouple the caller from that invariant by using IS_ERR_OR_NULL() and
> translating a NULL return to -ENOMEM, so a NULL can no longer be
> dereferenced regardless of how inetdev_init() signals failure.
>
> Cc: stable@vger.kernel.org
> Cc: syzkaller-bugs@googlegroups.com
> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
> ---
>
> Syzkaller reproducer:
> # {Threaded:true Repeat:true RepeatTimes:0 Procs:8 Slowdown:1 Sandbox:none SandboxArg:0 Leak:false NetInjection:false NetDevices:true NetReset:true Cgroups:true BinfmtMisc:true CloseFDs:true KCSAN:false DevlinkP
> CI:false NicVF:false USB:false VhciInjection:false Wifi:false IEEE802154:false Sysctl:true Swap:true UseTmpDir:true HandleSegv:true Trace:false CallComments:true LegacyOptions:{Collide:false Fault:false FaultCal
> l:0 FaultNth:0}}
> r0 = creat(&(0x7f00000000c0)='./file0\x00', 0x26)
> ioctl$RNDADDTOENTCNT(r0, 0x40045201, 0x0)
> r1 = socket$inet6_udplite(0xa, 0x2, 0x88)
> setsockopt$sock_int(r1, 0x1, 0x1d, &(0x7f0000000380), 0x4) (async, rerun: 64)
> unshare(0x42020000) (async, rerun: 64)
> truncate(&(0x7f0000000040)='./file0\x00', 0x9) (rerun: 64)
> ioctl$sock_SIOCBRADDBR(0xffffffffffffffff, 0x89a0, &(0x7f0000000000)='syzkaller0\x00')
> socket$inet_udplite(0x2, 0x2, 0x88)
> rename(&(0x7f0000000640)='./file0aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\x00', &(0x7f0000000780)='./file1\x00') (async)
> ioctl$BTRFS_IOC_BALANCE_PROGRESS(0xffffffffffffffff, 0x84009422, &(0x7f00000004c0)={0x0, 0x0, {0x0, @struct, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @struct}, {}, {0x0, @struct}})


inetdev_init() can not return NULL.

err is initialized to -ENOMEM, and the only place it is assigned is
inside "if (err)" after devinet_sysctl_register(). Therefore every path
reaching "return in_dev ?: ERR_PTR(err);" with in_dev == NULL has a
non-zero err, and IS_ERR() is sufficient.

Please do not send patches for bugs that can not happen, especially
with a Cc: stable and no Fixes: tag.

Also, the syzkaller reproducer you pasted has nothing to do with this
code path, and there is no syzbot report or Reported-by: here. Please
do not decorate patches with unrelated material.

If you used an LLM to generate this, please verify the claim before sending.

See Documentation/process/researcher-guidelines.rst

pw-bot: rejected

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ipv4: guard inetdev_event() against a NULL from inetdev_init()
  2026-09-21 16:09 ` Eric Dumazet
@ 2026-09-21 18:56   ` Yunseong Kim
  0 siblings, 0 replies; 3+ messages in thread
From: Yunseong Kim @ 2026-09-21 18:56 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, linux-kernel, David Ahern, Ido Schimmel, David S. Miller,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Taehee Yoo, stable,
	syzkaller-bugs

Hi Eric,

Thanks for the review.

On 21/09/2026 6:09 pm, Eric Dumazet wrote:
> On Mon, Sep 21, 2026 at 5:58 PM Yunseong Kim <yunseong.kim@est.tech> wrote:
>>
>> On NETDEV_REGISTER, inetdev_event() only checks the inetdev_init()
>> return value with IS_ERR():
>>
>>     in_dev = inetdev_init(dev);
>>     if (IS_ERR(in_dev))
>>         return notifier_from_errno(PTR_ERR(in_dev));
>>     if (dev->flags & IFF_LOOPBACK) {
>>         IN_DEV_CONF_SET(in_dev, NOXFRM, 1);
>>     ...
>>
>> Because IS_ERR(NULL) is false, a NULL return flows straight into
>> IN_DEV_CONF_SET() -> ipv4_devconf_set(), whose
>> set_bit(index, in_dev->cnf.state) dereferences NULL.
>>
>> inetdev_init() is only kept from returning NULL by its trailing
>>
>>     out:
>>         return in_dev ?: ERR_PTR(err);
>>
>> which relies on every failure path that leaves in_dev NULL having also
>> set a non-zero err. That invariant is fragile and lives in the producer,
>> while the consumer's IS_ERR()-only check silently depends on it: any
>> future inetdev_init() failure path that returns NULL (directly, or by
>> leaving err == 0) becomes a NULL dereference at this call site rather
>> than a clean error return.
>>
>> Before commit 20e61da7ffcf ("ipv4: fail early when creating netdev
>> named all or default") this call site used "if (!in_dev)", which caught
>> a NULL return; that commit converted inetdev_init() to the ERR_PTR()
>> convention and switched the check to IS_ERR(), dropping the NULL
>> handling here.
>>
>> Decouple the caller from that invariant by using IS_ERR_OR_NULL() and
>> translating a NULL return to -ENOMEM, so a NULL can no longer be
>> dereferenced regardless of how inetdev_init() signals failure.
>>
>> Cc: stable@vger.kernel.org
>> Cc: syzkaller-bugs@googlegroups.com
>> Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
>> ---
>>
>> Syzkaller reproducer:
>> # {Threaded:true Repeat:true RepeatTimes:0 Procs:8 Slowdown:1 Sandbox:none SandboxArg:0 Leak:false NetInjection:false NetDevices:true NetReset:true Cgroups:true BinfmtMisc:true CloseFDs:true KCSAN:false DevlinkP
>> CI:false NicVF:false USB:false VhciInjection:false Wifi:false IEEE802154:false Sysctl:true Swap:true UseTmpDir:true HandleSegv:true Trace:false CallComments:true LegacyOptions:{Collide:false Fault:false FaultCal
>> l:0 FaultNth:0}}
>> r0 = creat(&(0x7f00000000c0)='./file0\x00', 0x26)
>> ioctl$RNDADDTOENTCNT(r0, 0x40045201, 0x0)
>> r1 = socket$inet6_udplite(0xa, 0x2, 0x88)
>> setsockopt$sock_int(r1, 0x1, 0x1d, &(0x7f0000000380), 0x4) (async, rerun: 64)
>> unshare(0x42020000) (async, rerun: 64)
>> truncate(&(0x7f0000000040)='./file0\x00', 0x9) (rerun: 64)
>> ioctl$sock_SIOCBRADDBR(0xffffffffffffffff, 0x89a0, &(0x7f0000000000)='syzkaller0\x00')
>> socket$inet_udplite(0x2, 0x2, 0x88)
>> rename(&(0x7f0000000640)='./file0aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\x00', &(0x7f0000000780)='./file1\x00') (async)
>> ioctl$BTRFS_IOC_BALANCE_PROGRESS(0xffffffffffffffff, 0x84009422, &(0x7f00000004c0)={0x0, 0x0, {0x0, @struct, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @struct}, {}, {0x0, @struct}})
> 
> 
> inetdev_init() can not return NULL.

Okay, Thanks for pointing out.

> err is initialized to -ENOMEM, and the only place it is assigned is
> inside "if (err)" after devinet_sysctl_register(). Therefore every path
> reaching "return in_dev ?: ERR_PTR(err);" with in_dev == NULL has a
> non-zero err, and IS_ERR() is sufficient.

Ack, thanks again for the review. This issue is always reproducible, and I'm
currently trying to figure it out.

> Please do not send patches for bugs that can not happen, especially
> with a Cc: stable and no Fixes: tag.

Ack, I will take more time to why this bug happened.

> Also, the syzkaller reproducer you pasted has nothing to do with this
> code path, and there is no syzbot report or Reported-by: here. Please
> do not decorate patches with unrelated material.
> 
> If you used an LLM to generate this, please verify the claim before sending.

Oh, This isn't an LLM slob. My misunderstood the code problem. I didn't mean
to cause any extra burden by misusing the LLM. Sorry about that.

> See Documentation/process/researcher-guidelines.rst

Okay, I'll read the documentation more carefully.

> pw-bot: rejected

Thanks!

Best regards,
Yunseong

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-21 18:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 15:55 [PATCH] ipv4: guard inetdev_event() against a NULL from inetdev_init() Yunseong Kim
2026-09-21 16:09 ` Eric Dumazet
2026-09-21 18:56   ` Yunseong Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®