linux-kernel - Re: [syzbot] [net?] KASAN: slab-use-after-free Read in __ethtool_get_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iJwe-Q2Ve3O1OP4WTVuD6eawFvV+3eDvuvs4Xk=+j5yBg@mail.gmail.com>
Date: Mon, 30 Sep 2024 10:32:32 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Hillf Danton <hdanton@...a.com>
Cc: Denis Kirjanov <dkirjanov@...e.de>, 
	syzbot <syzbot+5fe14f2ff4ccbace9a26@...kaller.appspotmail.com>, 
	linux-kernel@...r.kernel.org, 
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>, Boqun Feng <boqun.feng@...il.com>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, netdev@...r.kernel.org, 
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [net?] KASAN: slab-use-after-free Read in __ethtool_get_link_ksettings

On Sun, Sep 29, 2024 at 1:46 PM Hillf Danton <hdanton@...a.com> wrote:
>
> On Sat, 28 Sep 2024 20:21:12 +0800 Hillf Danton <hdanton@...a.com>
> > On Mon, 25 Mar 2024 14:08:36 +0100 Eric Dumazet <edumazet@...gle.com>
> > > On Mon, Mar 25, 2024 at 1:10 PM Denis Kirjanov <dkirjanov@...e.de> wrote:
> > > >
> > > > Hmm, report says that we have a net_device freed even that we have a dev_hold()
> > > > before __ethtool_get_link_ksettings()
> > >
> > > dev_hold(dev) might be done too late, the device is already being dismantled.
> > >
> > > ib_device_get_netdev() should probably be done under RTNL locking,
> > > otherwise the final part is racy :
> > >
> > > if (res && res->reg_state != NETREG_REGISTERED) {
> > >      dev_put(res);
> > >      return NULL;
> > > }
> >
> > Given paranoia in netdev_run_todo(),
> >
> >               /* paranoia */
> >               BUG_ON(netdev_refcnt_read(dev) != 1);
> >
> > the claim that dev_hold(dev) might be done too late could not explain
> > the success of checking NETREG_REGISTERED, because of checking
> > NETREG_UNREGISTERING after rcu barrier.
> >
> >       /* Wait for rcu callbacks to finish before next phase */
> >       if (!list_empty(&list))
> >               rcu_barrier();
> >
> >       list_for_each_entry_safe(dev, tmp, &list, todo_list) {
> >               if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
> >                       netdev_WARN(dev, "run_todo but not unregistering\n");
> >                       list_del(&dev->todo_list);
> >                       continue;
> >               }
> >
> As simply bumping kref up could survive the syzbot reproducer [1], Eric's reclaim
> is incorrect.

I have about 50 different syzbot reports all pointing to netdevsim and
sysfs buggy interaction.

We will see if you can fix all of them :)

>
> --- l/drivers/infiniband/core/verbs.c
> +++ v/drivers/infiniband/core/verbs.c
> @@ -1979,6 +1979,7 @@ int ib_get_eth_speed(struct ib_device *d
>         netdev = ib_device_get_netdev(dev, port_num);
>         if (!netdev)
>                 return -ENODEV;
> +       dev_hold(netdev);
>
>         rtnl_lock();
>         rc = __ethtool_get_link_ksettings(netdev, &lksettings);
> @@ -1995,6 +1996,7 @@ int ib_get_eth_speed(struct ib_device *d
>                                 netdev->name, netdev_speed);
>         }
>
> +       dev_put(netdev);
>         ib_get_width_and_speed(netdev_speed, lksettings.lanes,
>                                speed, width);
>
> --
> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
>
> Reported-by: syzbot+5fe14f2ff4ccbace9a26@...kaller.appspotmail.com
> Tested-by: syzbot+5fe14f2ff4ccbace9a26@...kaller.appspotmail.com
>
> [1] https://lore.kernel.org/lkml/66f9372f.050a0220.aab67.001a.GAE@google.com/