netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net,v2 0/3] fix null pointer access issue in qdisc
@ 2022-10-18  6:31 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
                   ` (3 more replies)
  0 siblings, 4 replies; 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

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
---

Zhengchao Shao (3):
  net: sched: cake: fix null pointer access issue when cake_init() fails
  Revert "net: sched: fq_codel: remove redundant resource cleanup in
    fq_codel_init()"
  net: sched: sfb: fix null pointer access issue when sfb_init() fails

 net/sched/sch_cake.c     |  4 ++++
 net/sched/sch_fq_codel.c | 25 +++++++++++++++++--------
 net/sched/sch_sfb.c      |  3 ++-
 3 files changed, 23 insertions(+), 9 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [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

* [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 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

* 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

end of thread, other threads:[~2022-10-19 13:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).