Re: [syzbot] [net?] KASAN: slab-use-after-free Read in __ethtool_get_link_ksettings
From: Hillf Danton
Date: Sat Sep 28 2024 - 08:21:47 EST
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;
}