netdev - [Patch net-next] net_sched: properly check for empty skb array on error path

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 18 Dec 2017 14:34:26 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     netdev@...r.kernel.org
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        John Fastabend <john.fastabend@...il.com>
Subject: [Patch net-next] net_sched: properly check for empty skb array on error path

First, the check of &q->ring.queue against NULL is wrong, it
is always false. We should check the value rather than the address.

Secondly, we need the same check in pfifo_fast_reset() too,
as both ->reset() and ->destroy() are called in qdisc_destroy().

Fixes: c5ad119fb6c0 ("net: sched: pfifo_fast use skb_array")
Reported-by: syzbot <syzkaller@...glegroups.com>
Cc: John Fastabend <john.fastabend@...il.com>
Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
---
 net/sched/sch_generic.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 981c08fe810b..876fab2604b8 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -659,6 +659,12 @@ static void pfifo_fast_reset(struct Qdisc *qdisc)
 		struct skb_array *q = band2list(priv, band);
 		struct sk_buff *skb;
 
+		/* NULL ring is possible if destroy path is due to a failed
+		 * skb_array_init() in pfifo_fast_init() case.
+		 */
+		if (!q->ring.queue)
+			continue;
+
 		while ((skb = skb_array_consume_bh(q)) != NULL)
 			kfree_skb(skb);
 	}
@@ -719,7 +725,7 @@ static void pfifo_fast_destroy(struct Qdisc *sch)
 		/* NULL ring is possible if destroy path is due to a failed
 		 * skb_array_init() in pfifo_fast_init() case.
 		 */
-		if (!&q->ring.queue)
+		if (!q->ring.queue)
 			continue;
 		/* Destroy ring but no need to kfree_skb because a call to
 		 * pfifo_fast_reset() has already done that work.
-- 
2.13.0