| 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
| ||
|
Message-ID: <ZjE587MsVBZA61fJ@dcaratti.users.ipa.redhat.com>
Date: Tue, 30 Apr 2024 20:35:31 +0200
From: Davide Caratti <dcaratti@...hat.com>
To: Eric Dumazet <edumazet@...gle.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
hi Eric, thanks for looking at this!
On Tue, Apr 30, 2024 at 07:58:14PM +0200, Eric Dumazet wrote:
> On Tue, Apr 30, 2024 at 7:11 PM Davide Caratti <dcaratti@...hat.com> wrote:
> >
[...]
> > @@ -1389,6 +1389,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
> > ops->destroy(sch);
> > qdisc_put_stab(rtnl_dereference(sch->stab));
> > err_out3:
> > + lockdep_unregister_key(&sch->root_lock_key);
> > netdev_put(dev, &sch->dev_tracker);
> > qdisc_free(sch);
> > err_out2:
>
> 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:
BUG: sleeping function called from invalid context at kernel/locking/lockdep.c:6450
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0, name: swapper/6
preempt_count: 101, expected: 0
RCU nest depth: 0, expected: 0
1 lock held by swapper/6/0:
#0: ffffffff83e03000 (rcu_callback){....}-{0:0}, at: rcu_do_batch+0x1d6/0x630
Preemption disabled at:
[<0000000000000000>] 0x0
CPU: 6 PID: 0 Comm: swapper/6 Not tainted 6.9.0-rc2+ #655
Hardware name: Supermicro SYS-6027R-72RF/X9DRH-7TF/7F/iTF/iF, BIOS 3.0 07/26/2013
Call Trace:
<IRQ>
dump_stack_lvl+0xa9/0xc0
__might_resched+0x1a6/0x2b0
? rcu_do_batch+0x208/0x630
lockdep_unregister_key+0x28/0x290
qdisc_free+0x1b/0x40
rcu_do_batch+0x20d/0x630
? lockdep_hardirqs_on+0x78/0x100
rcu_core+0x305/0x570
__do_softirq+0xcd/0x484
irq_exit_rcu+0xc9/0x110
sysvec_apic_timer_interrupt+0x9e/0xc0
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x16/0x20
RIP: 0010:cpuidle_enter_state+0x104/0x5e0
Code: 00 89 c0 48 0f a3 05 3b 3e 1e 01 0f 82 4f 03 00 00 31 ff e8 5e 9e 33 ff 45 84 ff 0f 85 ff 02 00 00 e8 60 4b 47 ff fb 45 85 f6 <0f> 88 24 02 00 00 49 63 d6 48 2b 2c 24 48 8d 04 52 48 8d 04 82 49
RSP: 0018:ffffbe4ec22e7e78 EFLAGS: 00000202
RAX: 000000000002ec55 RBX: 0000000000000004 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff838d1436 RDI: ffffffff8385cace
RBP: 0000001486cedd9b R08: 0000000000000001 R09: 0000000000000001
R10: 00004d0a32102e4a R11: ffffffff83a0bb08 R12: ffffde4abd602710
R13: ffffffff83f66f20 R14: 0000000000000004 R15: 0000000000000000
6442 void lockdep_unregister_key(struct lock_class_key *key)
6443 {
6444 struct hlist_head *hash_head = keyhashentry(key);
6445 struct lock_class_key *k;
6446 struct pending_free *pf;
6447 unsigned long flags;
6448 bool found = false;
6449
6450 might_sleep();
... because of the above line. That's why in the normal path, the dynamic key is
unregistered before scheduling the RCU callback.
--
davide
Powered by blists - more mailing lists