* [PATCH net,v2 1/3] net: sched: cake: fix null pointer access issue when cake_init() fails
2022-10-18 6:31 [PATCH net,v2 0/3] fix null pointer access issue in qdisc Zhengchao Shao
@ 2022-10-18 6:31 ` Zhengchao Shao
2022-10-18 10:44 ` Toke Høiland-Jørgensen
2022-10-18 6:32 ` [PATCH net,v2 2/3] Revert "net: sched: fq_codel: remove redundant resource cleanup in fq_codel_init()" Zhengchao Shao
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Zhengchao Shao @ 2022-10-18 6:31 UTC (permalink / raw)
To: cake, netdev, toke, jhs, xiyou.wangcong, jiri, davem, edumazet,
kuba, pabeni
Cc: dave.taht, weiyongjun1, yuehaibing, shaozhengchao
When the default qdisc is cake, if the qdisc of dev_queue fails to be
inited during mqprio_init(), cake_reset() is invoked to clear
resources. In this case, the tins is NULL, and it will cause gpf issue.
The process is as follows:
qdisc_create_dflt()
cake_init()
q->tins = kvcalloc(...) --->failed, q->tins is NULL
...
qdisc_put()
...
cake_reset()
...
cake_dequeue_one()
b = &q->tins[...] --->q->tins is NULL
The following is the Call Trace information:
general protection fault, probably for non-canonical address
0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
RIP: 0010:cake_dequeue_one+0xc9/0x3c0
Call Trace:
<TASK>
cake_reset+0xb1/0x140
qdisc_reset+0xed/0x6f0
qdisc_destroy+0x82/0x4c0
qdisc_put+0x9e/0xb0
qdisc_create_dflt+0x2c3/0x4a0
mqprio_init+0xa71/0x1760
qdisc_create+0x3eb/0x1000
tc_modify_qdisc+0x408/0x1720
rtnetlink_rcv_msg+0x38e/0xac0
netlink_rcv_skb+0x12d/0x3a0
netlink_unicast+0x4a2/0x740
netlink_sendmsg+0x826/0xcc0
sock_sendmsg+0xc5/0x100
____sys_sendmsg+0x583/0x690
___sys_sendmsg+0xe8/0x160
__sys_sendmsg+0xbf/0x160
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7f89e5122d04
</TASK>
Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
net/sched/sch_cake.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 55c6879d2c7e..87f8ce2c65ee 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -2224,8 +2224,12 @@ static struct sk_buff *cake_dequeue(struct Qdisc *sch)
static void cake_reset(struct Qdisc *sch)
{
+ struct cake_sched_data *q = qdisc_priv(sch);
u32 c;
+ if (!q->tins)
+ return;
+
for (c = 0; c < CAKE_MAX_TINS; c++)
cake_clear_tin(sch, c);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net,v2 1/3] net: sched: cake: fix null pointer access issue when cake_init() fails
2022-10-18 6:31 ` [PATCH net,v2 1/3] net: sched: cake: fix null pointer access issue when cake_init() fails Zhengchao Shao
@ 2022-10-18 10:44 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-10-18 10:44 UTC (permalink / raw)
To: Zhengchao Shao, cake, netdev, jhs, xiyou.wangcong, jiri, davem,
edumazet, kuba, pabeni
Cc: dave.taht, weiyongjun1, yuehaibing, shaozhengchao
Zhengchao Shao <shaozhengchao@huawei.com> writes:
> When the default qdisc is cake, if the qdisc of dev_queue fails to be
> inited during mqprio_init(), cake_reset() is invoked to clear
> resources. In this case, the tins is NULL, and it will cause gpf issue.
>
> The process is as follows:
> qdisc_create_dflt()
> cake_init()
> q->tins = kvcalloc(...) --->failed, q->tins is NULL
> ...
> qdisc_put()
> ...
> cake_reset()
> ...
> cake_dequeue_one()
> b = &q->tins[...] --->q->tins is NULL
>
> The following is the Call Trace information:
> general protection fault, probably for non-canonical address
> 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> RIP: 0010:cake_dequeue_one+0xc9/0x3c0
> Call Trace:
> <TASK>
> cake_reset+0xb1/0x140
> qdisc_reset+0xed/0x6f0
> qdisc_destroy+0x82/0x4c0
> qdisc_put+0x9e/0xb0
> qdisc_create_dflt+0x2c3/0x4a0
> mqprio_init+0xa71/0x1760
> qdisc_create+0x3eb/0x1000
> tc_modify_qdisc+0x408/0x1720
> rtnetlink_rcv_msg+0x38e/0xac0
> netlink_rcv_skb+0x12d/0x3a0
> netlink_unicast+0x4a2/0x740
> netlink_sendmsg+0x826/0xcc0
> sock_sendmsg+0xc5/0x100
> ____sys_sendmsg+0x583/0x690
> ___sys_sendmsg+0xe8/0x160
> __sys_sendmsg+0xbf/0x160
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7f89e5122d04
> </TASK>
>
> Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net,v2 2/3] Revert "net: sched: fq_codel: remove redundant resource cleanup in fq_codel_init()"
2022-10-18 6:31 [PATCH net,v2 0/3] fix null pointer access issue in qdisc Zhengchao Shao
2022-10-18 6:31 ` [PATCH net,v2 1/3] net: sched: cake: fix null pointer access issue when cake_init() fails Zhengchao Shao
@ 2022-10-18 6:32 ` Zhengchao Shao
2022-10-18 6:32 ` [PATCH net,v2 3/3] net: sched: sfb: fix null pointer access issue when sfb_init() fails Zhengchao Shao
2022-10-19 13:00 ` [PATCH net,v2 0/3] fix null pointer access issue in qdisc patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Zhengchao Shao @ 2022-10-18 6:32 UTC (permalink / raw)
To: cake, netdev, toke, jhs, xiyou.wangcong, jiri, davem, edumazet,
kuba, pabeni
Cc: dave.taht, weiyongjun1, yuehaibing, shaozhengchao
This reverts commit 494f5063b86cd6e972cb41a27e083c9a3664319d.
When the default qdisc is fq_codel, if the qdisc of dev_queue fails to be
inited during mqprio_init(), fq_codel_reset() is invoked to clear
resources. In this case, the flow is NULL, and it will cause gpf issue.
The process is as follows:
qdisc_create_dflt()
fq_codel_init()
...
q->flows_cnt = 1024;
...
q->flows = kvcalloc(...) --->failed, q->flows is NULL
...
qdisc_put()
...
fq_codel_reset()
...
flow = q->flows + i --->q->flows is NULL
The following is the Call Trace information:
general protection fault, probably for non-canonical address
0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
RIP: 0010:fq_codel_reset+0x14d/0x350
Call Trace:
<TASK>
qdisc_reset+0xed/0x6f0
qdisc_destroy+0x82/0x4c0
qdisc_put+0x9e/0xb0
qdisc_create_dflt+0x2c3/0x4a0
mqprio_init+0xa71/0x1760
qdisc_create+0x3eb/0x1000
tc_modify_qdisc+0x408/0x1720
rtnetlink_rcv_msg+0x38e/0xac0
netlink_rcv_skb+0x12d/0x3a0
netlink_unicast+0x4a2/0x740
netlink_sendmsg+0x826/0xcc0
sock_sendmsg+0xc5/0x100
____sys_sendmsg+0x583/0x690
___sys_sendmsg+0xe8/0x160
__sys_sendmsg+0xbf/0x160
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7fd272b22d04
</TASK>
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
net/sched/sch_fq_codel.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 99d318b60568..8c4fee063436 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -478,24 +478,26 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
if (opt) {
err = fq_codel_change(sch, opt, extack);
if (err)
- return err;
+ goto init_failure;
}
err = tcf_block_get(&q->block, &q->filter_list, sch, extack);
if (err)
- return err;
+ goto init_failure;
if (!q->flows) {
q->flows = kvcalloc(q->flows_cnt,
sizeof(struct fq_codel_flow),
GFP_KERNEL);
- if (!q->flows)
- return -ENOMEM;
-
+ if (!q->flows) {
+ err = -ENOMEM;
+ goto init_failure;
+ }
q->backlogs = kvcalloc(q->flows_cnt, sizeof(u32), GFP_KERNEL);
- if (!q->backlogs)
- return -ENOMEM;
-
+ if (!q->backlogs) {
+ err = -ENOMEM;
+ goto alloc_failure;
+ }
for (i = 0; i < q->flows_cnt; i++) {
struct fq_codel_flow *flow = q->flows + i;
@@ -508,6 +510,13 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
else
sch->flags &= ~TCQ_F_CAN_BYPASS;
return 0;
+
+alloc_failure:
+ kvfree(q->flows);
+ q->flows = NULL;
+init_failure:
+ q->flows_cnt = 0;
+ return err;
}
static int fq_codel_dump(struct Qdisc *sch, struct sk_buff *skb)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net,v2 3/3] net: sched: sfb: fix null pointer access issue when sfb_init() fails
2022-10-18 6:31 [PATCH net,v2 0/3] fix null pointer access issue in qdisc Zhengchao Shao
2022-10-18 6:31 ` [PATCH net,v2 1/3] net: sched: cake: fix null pointer access issue when cake_init() fails Zhengchao Shao
2022-10-18 6:32 ` [PATCH net,v2 2/3] Revert "net: sched: fq_codel: remove redundant resource cleanup in fq_codel_init()" Zhengchao Shao
@ 2022-10-18 6:32 ` Zhengchao Shao
2022-10-19 13:00 ` [PATCH net,v2 0/3] fix null pointer access issue in qdisc patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Zhengchao Shao @ 2022-10-18 6:32 UTC (permalink / raw)
To: cake, netdev, toke, jhs, xiyou.wangcong, jiri, davem, edumazet,
kuba, pabeni
Cc: dave.taht, weiyongjun1, yuehaibing, shaozhengchao
When the default qdisc is sfb, if the qdisc of dev_queue fails to be
inited during mqprio_init(), sfb_reset() is invoked to clear resources.
In this case, the q->qdisc is NULL, and it will cause gpf issue.
The process is as follows:
qdisc_create_dflt()
sfb_init()
tcf_block_get() --->failed, q->qdisc is NULL
...
qdisc_put()
...
sfb_reset()
qdisc_reset(q->qdisc) --->q->qdisc is NULL
ops = qdisc->ops
The following is the Call Trace information:
general protection fault, probably for non-canonical address
0xdffffc0000000003: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
RIP: 0010:qdisc_reset+0x2b/0x6f0
Call Trace:
<TASK>
sfb_reset+0x37/0xd0
qdisc_reset+0xed/0x6f0
qdisc_destroy+0x82/0x4c0
qdisc_put+0x9e/0xb0
qdisc_create_dflt+0x2c3/0x4a0
mqprio_init+0xa71/0x1760
qdisc_create+0x3eb/0x1000
tc_modify_qdisc+0x408/0x1720
rtnetlink_rcv_msg+0x38e/0xac0
netlink_rcv_skb+0x12d/0x3a0
netlink_unicast+0x4a2/0x740
netlink_sendmsg+0x826/0xcc0
sock_sendmsg+0xc5/0x100
____sys_sendmsg+0x583/0x690
___sys_sendmsg+0xe8/0x160
__sys_sendmsg+0xbf/0x160
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7f2164122d04
</TASK>
Fixes: e13e02a3c68d ("net_sched: SFB flow scheduler")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
net/sched/sch_sfb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index e2389fa3cff8..73ae2e726512 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -455,7 +455,8 @@ static void sfb_reset(struct Qdisc *sch)
{
struct sfb_sched_data *q = qdisc_priv(sch);
- qdisc_reset(q->qdisc);
+ if (likely(q->qdisc))
+ qdisc_reset(q->qdisc);
q->slot = 0;
q->double_buffering = false;
sfb_zero_all_buckets(q);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net,v2 0/3] fix null pointer access issue in qdisc
2022-10-18 6:31 [PATCH net,v2 0/3] fix null pointer access issue in qdisc Zhengchao Shao
` (2 preceding siblings ...)
2022-10-18 6:32 ` [PATCH net,v2 3/3] net: sched: sfb: fix null pointer access issue when sfb_init() fails Zhengchao Shao
@ 2022-10-19 13:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-10-19 13:00 UTC (permalink / raw)
To: Zhengchao Shao
Cc: cake, netdev, toke, jhs, xiyou.wangcong, jiri, davem, edumazet,
kuba, pabeni, dave.taht, weiyongjun1, yuehaibing
Hello:
This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Tue, 18 Oct 2022 14:31:58 +0800 you wrote:
> These three patches fix the same type of problem. Set the default qdisc,
> and then construct an init failure scenario when the dev qdisc is
> configured on mqprio to trigger the reset process. NULL pointer access
> may occur during the reset process.
>
> ---
> v2: for fq_codel, revert the patch
>
> [...]
Here is the summary with links:
- [net,v2,1/3] net: sched: cake: fix null pointer access issue when cake_init() fails
https://git.kernel.org/netdev/net/c/51f9a8921cea
- [net,v2,2/3] Revert "net: sched: fq_codel: remove redundant resource cleanup in fq_codel_init()"
https://git.kernel.org/netdev/net/c/f5ffa3b11973
- [net,v2,3/3] net: sched: sfb: fix null pointer access issue when sfb_init() fails
https://git.kernel.org/netdev/net/c/2a3fc78210b9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread