[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20240929114601.1584-1-hdanton@sina.com>
Date: Sun, 29 Sep 2024 19:46:01 +0800
From: Hillf Danton <hdanton@...a.com>
To: Eric Dumazet <edumazet@...gle.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 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.
--- 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/
Powered by blists - more mailing lists