[PATCH net] net/sched: sch_taprio: do not schedule in taprio_reset()

[Eric Dumazet]

As reported by syzbot and hinted by Vinicius, I should not have added
a qdisc_synchronize() call in taprio_reset()

taprio_reset() can be called with qdisc spinlock held (and BH disabled)
as shown in included syzbot report [1].

Only taprio_destroy() needed this synchronization, as explained
in the blamed commit changelog.

[1]

BUG: scheduling while atomic: syz-executor150/5091/0x00000202
2 locks held by syz-executor150/5091:
Modules linked in:
Preemption disabled at:
[<0000000000000000>] 0x0
Kernel panic - not syncing: scheduling while atomic: panic_on_warn set ...
CPU: 1 PID: 5091 Comm: syz-executor150 Not tainted 6.2.0-rc3-syzkaller-00219-g010a74f52203 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd1/0x138 lib/dump_stack.c:106
panic+0x2cc/0x626 kernel/panic.c:318
check_panic_on_warn.cold+0x19/0x35 kernel/panic.c:238
__schedule_bug.cold+0xd5/0xfe kernel/sched/core.c:5836
schedule_debug kernel/sched/core.c:5865 [inline]
__schedule+0x34e4/0x5450 kernel/sched/core.c:6500
schedule+0xde/0x1b0 kernel/sched/core.c:6682
schedule_timeout+0x14e/0x2a0 kernel/time/timer.c:2167
schedule_timeout_uninterruptible kernel/time/timer.c:2201 [inline]
msleep+0xb6/0x100 kernel/time/timer.c:2322
qdisc_synchronize include/net/sch_generic.h:1295 [inline]
taprio_reset+0x93/0x270 net/sched/sch_taprio.c:1703
qdisc_reset+0x10c/0x770 net/sched/sch_generic.c:1022
dev_reset_queue+0x92/0x130 net/sched/sch_generic.c:1285
netdev_for_each_tx_queue include/linux/netdevice.h:2464 [inline]
dev_deactivate_many+0x36d/0x9f0 net/sched/sch_generic.c:1351
dev_deactivate+0xed/0x1b0 net/sched/sch_generic.c:1374
qdisc_graft+0xe4a/0x1380 net/sched/sch_api.c:1080
tc_modify_qdisc+0xb6b/0x19a0 net/sched/sch_api.c:1689
rtnetlink_rcv_msg+0x43e/0xca0 net/core/rtnetlink.c:6141
netlink_rcv_skb+0x165/0x440 net/netlink/af_netlink.c:2564
netlink_unicast_kernel net/netlink/af_netlink.c:1330 [inline]
netlink_unicast+0x547/0x7f0 net/netlink/af_netlink.c:1356
netlink_sendmsg+0x91b/0xe10 net/netlink/af_netlink.c:1932
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg+0xd3/0x120 net/socket.c:734
____sys_sendmsg+0x712/0x8c0 net/socket.c:2476
___sys_sendmsg+0x110/0x1b0 net/socket.c:2530
__sys_sendmsg+0xf7/0x1c0 net/socket.c:2559
do_syscall_x64 arch/x86/entry/common.c:50 [inline]

Fixes: 3a415d59c1db ("net/sched: sch_taprio: fix possible use-after-free")
Link: https://lore.kernel.org/netdev/167387581653.2747.13878941339...@kernel.org/T/
Reported-by: syzbot <syzk...@googlegroups.com>
Signed-off-by: Eric Dumazet <edum...@google.com>
Cc: Vinicius Costa Gomes <viniciu...@intel.com>
---
net/sched/sch_taprio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 9a11a499ea2df8d18c9c062496fdcbcf5a861391..c322a61eaeeac4b3744ec7b347d1256a19dfb244 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1700,7 +1700,6 @@ static void taprio_reset(struct Qdisc *sch)
int i;

hrtimer_cancel(&q->advance_timer);
- qdisc_synchronize(sch);

if (q->qdiscs) {
for (i = 0; i < dev->num_tx_queues; i++)
--
2.39.1.405.gd4c25cc71f-goog

[patchwork-b...@kernel.org]

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <ku...@kernel.org>:

On Mon, 23 Jan 2023 08:45:52 +0000 you wrote:
> As reported by syzbot and hinted by Vinicius, I should not have added
> a qdisc_synchronize() call in taprio_reset()
>
> taprio_reset() can be called with qdisc spinlock held (and BH disabled)
> as shown in included syzbot report [1].
>
> Only taprio_destroy() needed this synchronization, as explained
> in the blamed commit changelog.
>

> [...]

Here is the summary with links:
- [net] net/sched: sch_taprio: do not schedule in taprio_reset()
https://git.kernel.org/netdev/net/c/ea4fdbaa2f77

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[Greg Kroah-Hartman]

From: Eric Dumazet <edum...@google.com>

[ Upstream commit ea4fdbaa2f7798cb25adbe4fd52ffc6356f097bb ]

As reported by syzbot and hinted by Vinicius, I should not have added
a qdisc_synchronize() call in taprio_reset()

taprio_reset() can be called with qdisc spinlock held (and BH disabled)
as shown in included syzbot report [1].

Only taprio_destroy() needed this synchronization, as explained
in the blamed commit changelog.

Link: https://lore.kernel.org/r/20230123084552.5...@google.com
Signed-off-by: Jakub Kicinski <ku...@kernel.org>
Signed-off-by: Sasha Levin <sas...@kernel.org>

---
net/sched/sch_taprio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c

index 9a11a499ea2d..c322a61eaeea 100644

--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1700,7 +1700,6 @@ static void taprio_reset(struct Qdisc *sch)
int i;

hrtimer_cancel(&q->advance_timer);
- qdisc_synchronize(sch);

if (q->qdiscs) {
for (i = 0; i < dev->num_tx_queues; i++)
--

2.39.0

[Greg Kroah-Hartman]

index a76a2afe9585..135ea8b3816f 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1632,7 +1632,6 @@ static void taprio_reset(struct Qdisc *sch)

[Greg Kroah-Hartman]

index 5411bb4cdfc8..e25fe44899ff 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1621,7 +1621,6 @@ static void taprio_reset(struct Qdisc *sch)

[Greg Kroah-Hartman]

index 4278a466cb50..b7bd8c3e3158 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -1622,7 +1622,6 @@ static void taprio_reset(struct Qdisc *sch)

int i;

hrtimer_cancel(&q->advance_timer);
- qdisc_synchronize(sch);

if (q->qdiscs) {

for (i = 0; i < dev->num_tx_queues && q->qdiscs[i]; i++)
--
2.39.0