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

[Hillf Danton]

On Mon, 25 Mar 2024 14:08:36 +0100 Eric Dumazet <edumazet@xxxxxxxxxx>
> On Mon, Mar 25, 2024 at 1:10 PM Denis Kirjanov <dkirjanov@xxxxxxx> wrote:
> > On 3/22/24 23:10, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following issue on:
> > >
> > > HEAD commit:    61387b8dcf1d Merge tag 'for-6.9/dm-vdo' of git://git.kerne..
> > > git tree:       upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=11effbd1180000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=c6aea81bc9ff5e99
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=5fe14f2ff4ccbace9a26
> > > compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> > >
> > > Unfortunately, I don't have any reproducer for this issue yet.
> > >
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/b972a52930fa/disk-61387b8d.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/caa2592898b6/vmlinux-61387b8d.xz
> > > kernel image: https://storage.googleapis.com/syzbot-assets/4187257afcc5/bzImage-61387b8d.xz
> > >
> > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > > Reported-by: syzbot+5fe14f2ff4ccbace9a26@xxxxxxxxxxxxxxxxxxxxxxxxx
> > >
> > > ==================================================================
> > > BUG: KASAN: slab-use-after-free in __ethtool_get_link_ksettings+0x186/0x190 net/ethtool/ioctl.c:441
> > > Read of size 8 at addr ffff888021f46308 by task kworker/0:4/5169
> > >
> > > CPU: 0 PID: 5169 Comm: kworker/0:4 Not tainted 6.8.0-syzkaller-05562-g61387b8dcf1d #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024
> > > Workqueue: infiniband ib_cache_event_task
> > > Call Trace:
> > >  <TASK>
> > >  __dump_stack lib/dump_stack.c:88 [inline]
> > >  dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:114
> > >  print_address_description mm/kasan/report.c:377 [inline]
> > >  print_report+0xc3/0x620 mm/kasan/report.c:488
> > >  kasan_report+0xd9/0x110 mm/kasan/report.c:601
> > >  __ethtool_get_link_ksettings+0x186/0x190 net/ethtool/ioctl.c:441
> > >  __ethtool_get_link_ksettings+0xf5/0x190 net/ethtool/ioctl.c:445
> >
> > 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;
		}