diff mbox

net_sched: drr: check for NULL pointer in drr_dequeue

Message ID 1453951851-24501-1-git-send-email-bernie.harris@alliedtelesis.co.nz
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Bernie Harris Jan. 28, 2016, 3:30 a.m. UTC
There are cases where qdisc_dequeue_peeked can return NULL, and the result
is dereferenced later on in the function.

Similarly to the other qdisc dequeue functions, check whether the skb
pointer is NULL and if it is, goto out.

Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz>
---
 net/sched/sch_drr.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Sergei Shtylyov Jan. 28, 2016, 2:04 p.m. UTC | #1
Hello.

On 1/28/2016 6:30 AM, Bernie Harris wrote:

> There are cases where qdisc_dequeue_peeked can return NULL, and the result
> is dereferenced later on in the function.
>
> Similarly to the other qdisc dequeue functions, check whether the skb
> pointer is NULL and if it is, goto out.
>
> Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz>
> ---
>   net/sched/sch_drr.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
> index f26bdea..a1cd778 100644
> --- a/net/sched/sch_drr.c
> +++ b/net/sched/sch_drr.c
> @@ -403,6 +403,8 @@ static struct sk_buff *drr_dequeue(struct Qdisc *sch)
>   		if (len <= cl->deficit) {
>   			cl->deficit -= len;
>   			skb = qdisc_dequeue_peeked(cl->qdisc);
> +			if (unlikely(skb == NULL))

    !skb is preferred in the networking code. I think scripts/checkpatch.pl 
should've warned you.

[...]

MBR, Sergei
Cong Wang Jan. 29, 2016, 6:19 p.m. UTC | #2
On Wed, Jan 27, 2016 at 7:30 PM, Bernie Harris
<bernie.harris@alliedtelesis.co.nz> wrote:
> There are cases where qdisc_dequeue_peeked can return NULL, and the result
> is dereferenced later on in the function.
>
> Similarly to the other qdisc dequeue functions, check whether the skb
> pointer is NULL and if it is, goto out.
>
> Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz>

Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
David Miller Jan. 30, 2016, 1:27 a.m. UTC | #3
From: Bernie Harris <bernie.harris@alliedtelesis.co.nz>
Date: Thu, 28 Jan 2016 16:30:51 +1300

> There are cases where qdisc_dequeue_peeked can return NULL, and the result
> is dereferenced later on in the function.
> 
> Similarly to the other qdisc dequeue functions, check whether the skb
> pointer is NULL and if it is, goto out.
> 
> Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz>

Applied, thanks.
diff mbox

Patch

diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index f26bdea..a1cd778 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -403,6 +403,8 @@  static struct sk_buff *drr_dequeue(struct Qdisc *sch)
 		if (len <= cl->deficit) {
 			cl->deficit -= len;
 			skb = qdisc_dequeue_peeked(cl->qdisc);
+			if (unlikely(skb == NULL))
+				goto out;
 			if (cl->qdisc->q.qlen == 0)
 				list_del(&cl->alist);