netdev - Re: [PATCH net-next] net/sched: unregister lockdep keys in qdisc_create/qdisc

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] [day] [month] [year] [list]

Message-ID: <CANn89i+KVxvHkHT2c+L5ZGbeT34EO6HZahu4o9w9YB_YSJHH9w@mail.gmail.com>
Date: Fri, 3 May 2024 14:48:57 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Davide Caratti <dcaratti@...hat.com>
Cc: Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>, 
	Jiri Pirko <jiri@...nulli.us>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Naresh Kamboju <naresh.kamboju@...aro.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net/sched: unregister lockdep keys in
 qdisc_create/qdisc_alloc error path

On Fri, May 3, 2024 at 2:44 PM Davide Caratti <dcaratti@...hat.com> wrote:
>
> hello Eric,
>
> On Tue, Apr 30, 2024 at 08:43:22PM +0200, Eric Dumazet wrote:
> > On Tue, Apr 30, 2024 at 8:35 PM Davide Caratti <dcaratti@...hat.com> wrote:
> > >
>
> [...]
>
> > > > For consistency with the other path, what about this instead ?
> > > >
> > > > This would also  allow a qdisc goten from an rcu lookup to allow its
> > > > spinlock to be acquired.
> > > > (I am not saying this can happen, but who knows...)
> > > >
> > > > Ie defer the  lockdep_unregister_key() right before the kfree()
> > >
> > > the problem is, qdisc_free() is called also in a RCU callback. So, if we move
> > > lockdep_unregister_key() inside the function, the non-error path is
> > > going to splat like this
> >
> > Got it, but we do have ways of running a work queue after rcu grace period.
>
> this would imply scheduling a work that does qdisc_free() + lockdep_unregister_key()
> in qdisc_free_cb(). I can try that, but maybe the issue is different:
>
> > Let's use your patch, but I suspect we could have other issues.
> >
> > Full disclosure, I have the following syzbot report:
> >
> > WARNING: bad unlock balance detected!
> > 6.9.0-rc5-syzkaller-01413-gdd1941f801bc #0 Not tainted
> > -------------------------------------
> > kworker/u8:6/2474 is trying to release lock (&sch->root_lock_key) at:
> > [<ffffffff897300c5>] spin_unlock_bh include/linux/spinlock.h:396 [inline]
> > [<ffffffff897300c5>] dev_reset_queue+0x145/0x1b0 net/sched/sch_generic.c:1304
> > but there are no more locks to release!
>
> I don't understand how can this "imbalance" be caused by lockdep_unregister_key()
> being called too early. I'm more inclined to think that this splat is due to UaF
> similar to those that we saw a couples of days ago. Is syzbot still
> generating report like the one above?
>

 I had 22 other syzbot reports that I marked as a Duplicate of the original one.

Let's see if syzbot re-opens a new one after the fix.