Mon, Jul 29, 2019 at 10:00:18AM CEST, [email protected] 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.
>
>Fixes: 79bdc4c862af ("codel: generalize the implementation")

Looks like this is something being there since the beginning:
commit 76e3cc126bb223013a6b9a0e2a51238d1ef2e409
Author: Eric Dumazet <[email protected]>
Date:   Thu May 10 07:51:25 2012 +0000

    codel: Controlled Delay AQM


Please adjust "Fixes:".

The fix itself looks fine to me.
Reviewed-by: Jiri Pirko <[email protected]>



>Signed-off-by: Jia-Ju Bai <[email protected]>
>---
>v2:
>* Add a fix tag.
>  Thank Jiri Pirko for helpful advice.
>
>---
> 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
>

Reply via email to