Re: [syzbot] [net?] KASAN: slab-use-after-free Read in __ethtool_get_link_ksettings

[Hillf Danton <hdanton@sina.com>]

On Mon, 30 Sep 2024 10:32:32 +0200 Eric Dumazet <edumazet@google.com>
> On Sun, Sep 29, 2024 at 1:46 PM Hillf Danton <hdanton@sina.com> wrote:
> > On Sat, 28 Sep 2024 20:21:12 +0800 Hillf Danton <hdanton@sina.com>
> > > On Mon, 25 Mar 2024 14:08:36 +0100 Eric Dumazet <edumazet@google.com>
> > > > On Mon, Mar 25, 2024 at 1:10 PM Denis Kirjanov <dkirjanov@suse.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 :)
>
Looks like something I know
[1] https://lore.kernel.org/all/20220819220827.1639-1-hdanton@sina.com/
[2] https://lore.kernel.org/all/20231011211846.1345-1-hdanton@sina.com/
[3] https://lore.kernel.org/all/20240719112012.1562-1-hdanton@sina.com/

BTW I won the 2020 google OSPB Award (How many cents did you donate that year?).

And my current target is the Independent Observer at the 2025 Linux Plumbers
Conference if the LT king thinks observers are welcome.

Now turn to uaf. As the netdev_hold() in ib_device_set_netdev() matches the
netdev_put() in free_netdevs(), in combination with the dev_hold() in
ib_device_get_netdev(), the syz report discovers a valid case where kref fails
to prevent netdev from being freed under feet, even given the paranoia
in netdev_run_todo().

Hillf