Skip to content

Commit 9efd232

Browse files
tohojodavem330
authored andcommitted
sch_sfb: Don't assume the skb is still around after enqueueing to child
The sch_sfb enqueue() routine assumes the skb is still alive after it has been enqueued into a child qdisc, using the data in the skb cb field in the increment_qlen() routine after enqueue. However, the skb may in fact have been freed, causing a use-after-free in this case. In particular, this happens if sch_cake is used as a child of sfb, and the GSO splitting mode of CAKE is enabled (in which case the skb will be split into segments and the original skb freed). Fix this by copying the sfb cb data to the stack before enqueueing the skb, and using this stack copy in increment_qlen() instead of the skb pointer itself. Reported-by: [email protected] # ZDI-CAN-18231 Fixes: e13e02a ("net_sched: SFB flow scheduler") Signed-off-by: Toke Høiland-Jørgensen <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 7fdc776 commit 9efd232

File tree

1 file changed

+6
-4
lines changed

1 file changed

+6
-4
lines changed

net/sched/sch_sfb.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,15 @@ static void increment_one_qlen(u32 sfbhash, u32 slot, struct sfb_sched_data *q)
135135
}
136136
}
137137

138-
static void increment_qlen(const struct sk_buff *skb, struct sfb_sched_data *q)
138+
static void increment_qlen(const struct sfb_skb_cb *cb, struct sfb_sched_data *q)
139139
{
140140
u32 sfbhash;
141141

142-
sfbhash = sfb_hash(skb, 0);
142+
sfbhash = cb->hashes[0];
143143
if (sfbhash)
144144
increment_one_qlen(sfbhash, 0, q);
145145

146-
sfbhash = sfb_hash(skb, 1);
146+
sfbhash = cb->hashes[1];
147147
if (sfbhash)
148148
increment_one_qlen(sfbhash, 1, q);
149149
}
@@ -283,6 +283,7 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
283283
struct sfb_sched_data *q = qdisc_priv(sch);
284284
struct Qdisc *child = q->qdisc;
285285
struct tcf_proto *fl;
286+
struct sfb_skb_cb cb;
286287
int i;
287288
u32 p_min = ~0;
288289
u32 minqlen = ~0;
@@ -399,11 +400,12 @@ static int sfb_enqueue(struct sk_buff *skb, struct Qdisc *sch,
399400
}
400401

401402
enqueue:
403+
memcpy(&cb, sfb_skb_cb(skb), sizeof(cb));
402404
ret = qdisc_enqueue(skb, child, to_free);
403405
if (likely(ret == NET_XMIT_SUCCESS)) {
404406
qdisc_qstats_backlog_inc(sch, skb);
405407
sch->q.qlen++;
406-
increment_qlen(skb, q);
408+
increment_qlen(&cb, q);
407409
} else if (net_xmit_drop_count(ret)) {
408410
q->stats.childdrop++;
409411
qdisc_qstats_drop(sch);

0 commit comments

Comments
 (0)