net/sched: sch_cbs: Call qdisc_reset for child qdisc
During a reset, CBS is not calling reset on its child qdisc, which
might cause qlen/backlog accounting issues. For example, if we have CBS
with a QFQ parent and a netem child with delay, we can create a scenario
where the parent's qlen underflows. QFQ, specifically, uses qlen to
check whether it should deference a pointer, so this scenario may cause
a null-ptr deref in QFQ:
[ 43.875639][ T319] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000009: 0000 [#1] SMP KASAN NOPTI
[ 43.876124][ T319] KASAN: null-ptr-deref in range [0x0000000000000048-0x000000000000004f]
[ 43.876417][ T319] CPU: 10 UID: 0 PID: 319 Comm: ping Not tainted
7.0.0-13039-ge728258debd5 #773 PREEMPT(full)
[ 43.876751][ T319] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 43.876949][ T319] RIP: 0010:qfq_dequeue+0x35c/0x1650
[ 43.877123][ T319] Code: 00 fc ff df 80 3c 02 00 0f 85 17 0e 00 00 4c 8d 73 48 48 89 9d b8 02 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <80> 3c 02 00 0f 85 76 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b
[ 43.877648][ T319] RSP: 0018:
ffff8881017ef4f0 EFLAGS:
00010216
[ 43.877845][ T319] RAX:
dffffc0000000000 RBX:
0000000000000000 RCX:
dffffc0000000000
[ 43.878073][ T319] RDX:
0000000000000009 RSI:
0000000c40000000 RDI:
ffff88810eef02b0
[ 43.878306][ T319] RBP:
ffff88810eef0000 R08:
ffff88810eef0280 R09:
1ffff1102120fd63
[ 43.878523][ T319] R10:
1ffff1102120fd66 R11:
1ffff1102120fd67 R12:
0000000c40000000
[ 43.878742][ T319] R13:
ffff88810eef02b8 R14:
0000000000000048 R15:
0000000020000000
[ 43.878959][ T319] FS:
00007f9c51c47c40(0000) GS:
ffff88817a0be000(0000) knlGS:
0000000000000000
[ 43.879214][ T319] CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
[ 43.879403][ T319] CR2:
000055e69a2230a8 CR3:
000000010c07a000 CR4:
0000000000750ef0
[ 43.879621][ T319] PKRU:
55555554
[ 43.879735][ T319] Call Trace:
[ 43.879844][ T319] <TASK>
[ 43.879924][ T319] __qdisc_run+0x169/0x1900
[ 43.880075][ T319] ? dev_qdisc_enqueue+0x8b/0x210
[ 43.880222][ T319] __dev_queue_xmit+0x2346/0x37a0
[ 43.880376][ T319] ? register_lock_class+0x3f/0x800
[ 43.880531][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.880684][ T319] ? __pfx___dev_queue_xmit+0x10/0x10
[ 43.880834][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.880977][ T319] ? __lock_acquire+0x819/0x1df0
[ 43.881124][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.881275][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.881418][ T319] ? __asan_memcpy+0x3c/0x60
[ 43.881563][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.881708][ T319] ? eth_header+0x165/0x1a0
[ 43.881853][ T319] ? lockdep_hardirqs_on_prepare+0xdb/0x1a0
[ 43.882031][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.882174][ T319] ? neigh_resolve_output+0x3cc/0x7e0
[ 43.882325][ T319] ? srso_alias_return_thunk+0x5/0xfbef5
[ 43.882471][ T319] ip_finish_output2+0x6b6/0x1e10
Fix this by calling qdisc_reset for CBS' child qdisc.
Sashiko caught an issue which could result in a null ptr deref if
qdisc_create_dflt() is invoked on an unitialised cbs qdisc which is exposed
by this patch. We add an early return if the qdisc is null to address this.
This is a similar approach used by two other fixes[1][2].
The proper fix for this specific issue elucidated by sashiko is to remove
the call to qdisc_reset when qdisc_create_dflt fails. Since the dflt qdisc
isn't attached anywhere yet at that point, calling the reset callback doesn't
make much sense (and as stated has been a source of two other bugs).
We plan on submitting this fix in a later patch.
[1] https://lore.kernel.org/netdev/
20221018063201.306474-2-shaozhengchao@huawei.com/
[2] https://lore.kernel.org/netdev/
20221018063201.306474-4-shaozhengchao@huawei.com/
Fixes: 585d763af09c ("net/sched: Introduce Credit Based Shaper (CBS) qdisc")
Reported-by: Junyoung Jang <graypanda.inzag@gmail.com>
Tested-by: Junyoung Jang <graypanda.inzag@gmail.com>
Tested-by: Victor Nogueira <victor@mojatatu.com>
Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>