Message ID | 20190729022157.18090-1-baijiaju1990@gmail.com |
---|---|
State | Superseded |
Delegated to: | David Miller |
Headers | show |
Series | net: sched: Fix a possible null-pointer dereference in dequeue_func() | expand |
Mon, Jul 29, 2019 at 04:21:57AM CEST, baijiaju1990@gmail.com wrote: >In dequeue_func(), there is an if statement on line 74 to check whether >skb is NULL: > if (skb) > >When skb is NULL, it is used on line 77: > prefetch(&skb->end); > >Thus, a possible null-pointer dereference may occur. > >To fix this bug, skb->end is used when skb is not NULL. > >This bug is found by a static analysis tool STCheck written by us. > >Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> Fixes tag, please? >--- > net/sched/sch_codel.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c >index 25ef172c23df..30169b3adbbb 100644 >--- a/net/sched/sch_codel.c >+++ b/net/sched/sch_codel.c >@@ -71,10 +71,10 @@ static struct sk_buff *dequeue_func(struct codel_vars *vars, void *ctx) > struct Qdisc *sch = ctx; > struct sk_buff *skb = __qdisc_dequeue_head(&sch->q); > >- if (skb) >+ if (skb) { > sch->qstats.backlog -= qdisc_pkt_len(skb); >- >- prefetch(&skb->end); /* we'll need skb_shinfo() */ >+ prefetch(&skb->end); /* we'll need skb_shinfo() */ >+ } > return skb; > } > >-- >2.17.0 >
On 2019/7/29 14:56, Jiri Pirko wrote: > Mon, Jul 29, 2019 at 04:21:57AM CEST, baijiaju1990@gmail.com wrote: >> In dequeue_func(), there is an if statement on line 74 to check whether >> skb is NULL: >> if (skb) >> >> When skb is NULL, it is used on line 77: >> prefetch(&skb->end); >> >> Thus, a possible null-pointer dereference may occur. >> >> To fix this bug, skb->end is used when skb is not NULL. >> >> This bug is found by a static analysis tool STCheck written by us. >> >> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> > Fixes tag, please? Sorry, I do not know what "fixes tag" means... I just find a possible bug and fix it in this patch. Best wishes, Jia-Ju Bai
Mon, Jul 29, 2019 at 09:32:00AM CEST, baijiaju1990@gmail.com wrote: > > >On 2019/7/29 14:56, Jiri Pirko wrote: >> Mon, Jul 29, 2019 at 04:21:57AM CEST, baijiaju1990@gmail.com wrote: >> > In dequeue_func(), there is an if statement on line 74 to check whether >> > skb is NULL: >> > if (skb) >> > >> > When skb is NULL, it is used on line 77: >> > prefetch(&skb->end); >> > >> > Thus, a possible null-pointer dereference may occur. >> > >> > To fix this bug, skb->end is used when skb is not NULL. >> > >> > This bug is found by a static analysis tool STCheck written by us. >> > >> > Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> >> Fixes tag, please? > >Sorry, I do not know what "fixes tag" means... >I just find a possible bug and fix it in this patch. git log |grep Fixes: If A fix goes to -net tree, it most probably fixes some bug introduced by some commit in the past. So this tag is to put a reference. > > >Best wishes, >Jia-Ju Bai
On 2019/7/29 15:41, Jiri Pirko wrote: > Mon, Jul 29, 2019 at 09:32:00AM CEST, baijiaju1990@gmail.com wrote: >> >> On 2019/7/29 14:56, Jiri Pirko wrote: >>> Mon, Jul 29, 2019 at 04:21:57AM CEST, baijiaju1990@gmail.com wrote: >>>> In dequeue_func(), there is an if statement on line 74 to check whether >>>> skb is NULL: >>>> if (skb) >>>> >>>> When skb is NULL, it is used on line 77: >>>> prefetch(&skb->end); >>>> >>>> Thus, a possible null-pointer dereference may occur. >>>> >>>> To fix this bug, skb->end is used when skb is not NULL. >>>> >>>> This bug is found by a static analysis tool STCheck written by us. >>>> >>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> >>> Fixes tag, please? >> Sorry, I do not know what "fixes tag" means... >> I just find a possible bug and fix it in this patch. > git log |grep Fixes: > > If A fix goes to -net tree, it most probably fixes some bug introduced > by some commit in the past. So this tag is to put a reference. Thanks for the explanation. I will add it and submit a v2 patch. Best wishes, Jia-Ju Bai
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c index 25ef172c23df..30169b3adbbb 100644 --- a/net/sched/sch_codel.c +++ b/net/sched/sch_codel.c @@ -71,10 +71,10 @@ static struct sk_buff *dequeue_func(struct codel_vars *vars, void *ctx) struct Qdisc *sch = ctx; struct sk_buff *skb = __qdisc_dequeue_head(&sch->q); - if (skb) + if (skb) { sch->qstats.backlog -= qdisc_pkt_len(skb); - - prefetch(&skb->end); /* we'll need skb_shinfo() */ + prefetch(&skb->end); /* we'll need skb_shinfo() */ + } return skb; }
In dequeue_func(), there is an if statement on line 74 to check whether skb is NULL: if (skb) When skb is NULL, it is used on line 77: prefetch(&skb->end); Thus, a possible null-pointer dereference may occur. To fix this bug, skb->end is used when skb is not NULL. This bug is found by a static analysis tool STCheck written by us. Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- net/sched/sch_codel.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)