sched: Avoid dereferencing skb pointer after child enqueue

tohojo · davem330 · commit f6bab199315b · 2019-01-15T20:12:00.000-08:00
Parent qdiscs may dereference the pointer to the enqueued skb after
enqueue. However, both CAKE and TBF call consume_skb() on the original skb
when splitting GSO packets, leading to a potential use-after-free in the
parent. Fix this by avoiding dereferencing the skb pointer after enqueueing
to the child.

Signed-off-by: Toke Høiland-Jørgensen &lt;toke@redhat.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
@@ -88,13 +88,14 @@ static int cbs_child_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			     struct Qdisc *child,
 			     struct sk_buff **to_free)
 {
+	unsigned int len = qdisc_pkt_len(skb);
 	int err;
 
 	err = child->ops->enqueue(skb, child, to_free);
 	if (err != NET_XMIT_SUCCESS)
 		return err;
 
-	qdisc_qstats_backlog_inc(sch, skb);
+	sch->qstats.backlog += len;
 	sch->q.qlen++;
 
 	return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
@@ -350,6 +350,7 @@ static struct drr_class *drr_classify(struct sk_buff *skb, struct Qdisc *sch,
 static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		       struct sk_buff **to_free)
 {
+	unsigned int len = qdisc_pkt_len(skb);
 	struct drr_sched *q = qdisc_priv(sch);
 	struct drr_class *cl;
 	int err = 0;
@@ -376,7 +377,7 @@ static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		cl->deficit = cl->quantum;
 	}
 
-	qdisc_qstats_backlog_inc(sch, skb);
+	sch->qstats.backlog += len;
 	sch->q.qlen++;
 	return err;
 }
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
@@ -199,6 +199,7 @@ static struct tcf_block *dsmark_tcf_block(struct Qdisc *sch, unsigned long cl,
 static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 			  struct sk_buff **to_free)
 {
+	unsigned int len = qdisc_pkt_len(skb);
 	struct dsmark_qdisc_data *p = qdisc_priv(sch);
 	int err;
 
@@ -271,7 +272,7 @@ static int dsmark_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		return err;
 	}
 
-	qdisc_qstats_backlog_inc(sch, skb);
+	sch->qstats.backlog += len;
 	sch->q.qlen++;
 
 	return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
@@ -1539,6 +1539,7 @@ hfsc_dump_qdisc(struct Qdisc *sch, struct sk_buff *skb)
 static int
 hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 {
+	unsigned int len = qdisc_pkt_len(skb);
 	struct hfsc_class *cl;
 	int uninitialized_var(err);
 
@@ -1560,8 +1561,6 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 	}
 
 	if (cl->qdisc->q.qlen == 1) {
-		unsigned int len = qdisc_pkt_len(skb);
-
 		if (cl->cl_flags & HFSC_RSC)
 			init_ed(cl, len);
 		if (cl->cl_flags & HFSC_FSC)
@@ -1576,7 +1575,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 
 	}
 
-	qdisc_qstats_backlog_inc(sch, skb);
+	sch->qstats.backlog += len;
 	sch->q.qlen++;
 
 	return NET_XMIT_SUCCESS;
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
@@ -581,6 +581,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		       struct sk_buff **to_free)
 {
 	int uninitialized_var(ret);
+	unsigned int len = qdisc_pkt_len(skb);
 	struct htb_sched *q = qdisc_priv(sch);
 	struct htb_class *cl = htb_classify(skb, sch, &ret);
 
@@ -610,7 +611,7 @@ static int htb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		htb_activate(q, cl);
 	}
 
-	qdisc_qstats_backlog_inc(sch, skb);
+	sch->qstats.backlog += len;
 	sch->q.qlen++;
 	return NET_XMIT_SUCCESS;
 }
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
@@ -72,6 +72,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 static int
 prio_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 {
+	unsigned int len = qdisc_pkt_len(skb);
 	struct Qdisc *qdisc;
 	int ret;
 
@@ -88,7 +89,7 @@ prio_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
 
 	ret = qdisc_enqueue(skb, qdisc, to_free);
 	if (ret == NET_XMIT_SUCCESS) {
-		qdisc_qstats_backlog_inc(sch, skb);
+		sch->qstats.backlog += len;
 		sch->q.qlen++;
 		return NET_XMIT_SUCCESS;
 	}
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
@@ -1210,6 +1210,7 @@ static struct qfq_aggregate *qfq_choose_next_agg(struct qfq_sched *q)
 static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		       struct sk_buff **to_free)
 {
+	unsigned int len = qdisc_pkt_len(skb), gso_segs;
 	struct qfq_sched *q = qdisc_priv(sch);
 	struct qfq_class *cl;
 	struct qfq_aggregate *agg;
@@ -1224,17 +1225,17 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	}
 	pr_debug("qfq_enqueue: cl = %x\n", cl->common.classid);
 
-	if (unlikely(cl->agg->lmax < qdisc_pkt_len(skb))) {
+	if (unlikely(cl->agg->lmax < len)) {
 		pr_debug("qfq: increasing maxpkt from %u to %u for class %u",
-			 cl->agg->lmax, qdisc_pkt_len(skb), cl->common.classid);
-		err = qfq_change_agg(sch, cl, cl->agg->class_weight,
-				     qdisc_pkt_len(skb));
+			 cl->agg->lmax, len, cl->common.classid);
+		err = qfq_change_agg(sch, cl, cl->agg->class_weight, len);
 		if (err) {
 			cl->qstats.drops++;
 			return qdisc_drop(skb, sch, to_free);
 		}
 	}
 
+	gso_segs = skb_is_gso(skb) ? skb_shinfo(skb)->gso_segs : 1;
 	err = qdisc_enqueue(skb, cl->qdisc, to_free);
 	if (unlikely(err != NET_XMIT_SUCCESS)) {
 		pr_debug("qfq_enqueue: enqueue failed %d\n", err);
@@ -1245,16 +1246,17 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		return err;
 	}
 
-	bstats_update(&cl->bstats, skb);
-	qdisc_qstats_backlog_inc(sch, skb);
+	cl->bstats.bytes += len;
+	cl->bstats.packets += gso_segs;
+	sch->qstats.backlog += len;
 	++sch->q.qlen;
 
 	agg = cl->agg;
 	/* if the queue was not empty, then done here */
 	if (cl->qdisc->q.qlen != 1) {
 		if (unlikely(skb == cl->qdisc->ops->peek(cl->qdisc)) &&
 		    list_first_entry(&agg->active, struct qfq_class, alist)
-		    == cl && cl->deficit < qdisc_pkt_len(skb))
+		    == cl && cl->deficit < len)
 			list_move_tail(&cl->alist, &agg->active);
 
 		return err;
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
@@ -185,6 +185,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		       struct sk_buff **to_free)
 {
 	struct tbf_sched_data *q = qdisc_priv(sch);
+	unsigned int len = qdisc_pkt_len(skb);
 	int ret;
 
 	if (qdisc_pkt_len(skb) > q->max_size) {
@@ -200,7 +201,7 @@ static int tbf_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 		return ret;
 	}
 
-	qdisc_qstats_backlog_inc(sch, skb);
+	sch->qstats.backlog += len;
 	sch->q.qlen++;
 	return NET_XMIT_SUCCESS;
 }

Original file line number	Diff line number	Diff line change
`@@ -350,6 +350,7 @@ static struct drr_class drr_classify(struct sk_buff skb, struct Qdisc *sch,`
`350`	`350`	`static int drr_enqueue(struct sk_buff skb, struct Qdisc sch,`
`351`	`351`	`struct sk_buff **to_free)`
`352`	`352`	`{`
	`353`	`+ unsigned int len = qdisc_pkt_len(skb);`
`353`	`354`	`struct drr_sched *q = qdisc_priv(sch);`
`354`	`355`	`struct drr_class *cl;`
`355`	`356`	`int err = 0;`
`@@ -376,7 +377,7 @@ static int drr_enqueue(struct sk_buff skb, struct Qdisc sch,`
`376`	`377`	`cl->deficit = cl->quantum;`
`377`	`378`	`}`
`378`	`379`
`379`		`- qdisc_qstats_backlog_inc(sch, skb);`
	`380`	`+ sch->qstats.backlog += len;`
`380`	`381`	`sch->q.qlen++;`
`381`	`382`	`return err;`
`382`	`383`	`}`
Original file line number	Diff line number	Diff line change
`@@ -199,6 +199,7 @@ static struct tcf_block dsmark_tcf_block(struct Qdisc sch, unsigned long cl,`
`199`	`199`	`static int dsmark_enqueue(struct sk_buff skb, struct Qdisc sch,`
`200`	`200`	`struct sk_buff **to_free)`
`201`	`201`	`{`
	`202`	`+ unsigned int len = qdisc_pkt_len(skb);`
`202`	`203`	`struct dsmark_qdisc_data *p = qdisc_priv(sch);`
`203`	`204`	`int err;`
`204`	`205`
`@@ -271,7 +272,7 @@ static int dsmark_enqueue(struct sk_buff skb, struct Qdisc sch,`
`271`	`272`	`return err;`
`272`	`273`	`}`
`273`	`274`
`274`		`- qdisc_qstats_backlog_inc(sch, skb);`
	`275`	`+ sch->qstats.backlog += len;`
`275`	`276`	`sch->q.qlen++;`
`276`	`277`
`277`	`278`	`return NET_XMIT_SUCCESS;`
Original file line number	Diff line number	Diff line change
`@@ -1539,6 +1539,7 @@ hfsc_dump_qdisc(struct Qdisc sch, struct sk_buff skb)`
`1539`	`1539`	`static int`
`1540`	`1540`	`hfsc_enqueue(struct sk_buff skb, struct Qdisc sch, struct sk_buff **to_free)`
`1541`	`1541`	`{`
	`1542`	`+ unsigned int len = qdisc_pkt_len(skb);`
`1542`	`1543`	`struct hfsc_class *cl;`
`1543`	`1544`	`int uninitialized_var(err);`
`1544`	`1545`
`@@ -1560,8 +1561,6 @@ hfsc_enqueue(struct sk_buff skb, struct Qdisc sch, struct sk_buff **to_free)`
`1560`	`1561`	`}`
`1561`	`1562`
`1562`	`1563`	`if (cl->qdisc->q.qlen == 1) {`
`1563`		`- unsigned int len = qdisc_pkt_len(skb);`
`1564`		`-`
`1565`	`1564`	`if (cl->cl_flags & HFSC_RSC)`
`1566`	`1565`	`init_ed(cl, len);`
`1567`	`1566`	`if (cl->cl_flags & HFSC_FSC)`
`@@ -1576,7 +1575,7 @@ hfsc_enqueue(struct sk_buff skb, struct Qdisc sch, struct sk_buff **to_free)`
`1576`	`1575`
`1577`	`1576`	`}`
`1578`	`1577`
`1579`		`- qdisc_qstats_backlog_inc(sch, skb);`
	`1578`	`+ sch->qstats.backlog += len;`
`1580`	`1579`	`sch->q.qlen++;`
`1581`	`1580`
`1582`	`1581`	`return NET_XMIT_SUCCESS;`
Original file line number	Diff line number	Diff line change
`@@ -581,6 +581,7 @@ static int htb_enqueue(struct sk_buff skb, struct Qdisc sch,`
`581`	`581`	`struct sk_buff **to_free)`
`582`	`582`	`{`
`583`	`583`	`int uninitialized_var(ret);`
	`584`	`+ unsigned int len = qdisc_pkt_len(skb);`
`584`	`585`	`struct htb_sched *q = qdisc_priv(sch);`
`585`	`586`	`struct htb_class *cl = htb_classify(skb, sch, &ret);`
`586`	`587`
`@@ -610,7 +611,7 @@ static int htb_enqueue(struct sk_buff skb, struct Qdisc sch,`
`610`	`611`	`htb_activate(q, cl);`
`611`	`612`	`}`
`612`	`613`
`613`		`- qdisc_qstats_backlog_inc(sch, skb);`
	`614`	`+ sch->qstats.backlog += len;`
`614`	`615`	`sch->q.qlen++;`
`615`	`616`	`return NET_XMIT_SUCCESS;`
`616`	`617`	`}`
Original file line number	Diff line number	Diff line change
`@@ -72,6 +72,7 @@ prio_classify(struct sk_buff skb, struct Qdisc sch, int *qerr)`
`72`	`72`	`static int`
`73`	`73`	`prio_enqueue(struct sk_buff skb, struct Qdisc sch, struct sk_buff **to_free)`
`74`	`74`	`{`
	`75`	`+ unsigned int len = qdisc_pkt_len(skb);`
`75`	`76`	`struct Qdisc *qdisc;`
`76`	`77`	`int ret;`
`77`	`78`
`@@ -88,7 +89,7 @@ prio_enqueue(struct sk_buff skb, struct Qdisc sch, struct sk_buff **to_free)`
`88`	`89`
`89`	`90`	`ret = qdisc_enqueue(skb, qdisc, to_free);`
`90`	`91`	`if (ret == NET_XMIT_SUCCESS) {`
`91`		`- qdisc_qstats_backlog_inc(sch, skb);`
	`92`	`+ sch->qstats.backlog += len;`
`92`	`93`	`sch->q.qlen++;`
`93`	`94`	`return NET_XMIT_SUCCESS;`
`94`	`95`	`}`
Original file line number	Diff line number	Diff line change
`@@ -185,6 +185,7 @@ static int tbf_enqueue(struct sk_buff skb, struct Qdisc sch,`
`185`	`185`	`struct sk_buff **to_free)`
`186`	`186`	`{`
`187`	`187`	`struct tbf_sched_data *q = qdisc_priv(sch);`
	`188`	`+ unsigned int len = qdisc_pkt_len(skb);`
`188`	`189`	`int ret;`
`189`	`190`
`190`	`191`	`if (qdisc_pkt_len(skb) > q->max_size) {`
`@@ -200,7 +201,7 @@ static int tbf_enqueue(struct sk_buff skb, struct Qdisc sch,`
`200`	`201`	`return ret;`
`201`	`202`	`}`
`202`	`203`
`203`		`- qdisc_qstats_backlog_inc(sch, skb);`
	`204`	`+ sch->qstats.backlog += len;`
`204`	`205`	`sch->q.qlen++;`
`205`	`206`	`return NET_XMIT_SUCCESS;`
`206`	`207`	`}`