* [PATCH net] qfq: don't leak skb if kzalloc fails
@ 2016-06-08 12:31 Florian Westphal
2016-06-08 15:54 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Florian Westphal @ 2016-06-08 12:31 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
When we need to create a new aggregate to enqueue the skb we need
to kzalloc the new struct.
If that fails we returned ENOBUFS without freeing the skb.
Spotted during code review.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/sched/sch_qfq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 8d2d8d9..435b970 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -1236,7 +1236,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
err = qfq_change_agg(sch, cl, cl->agg->class_weight,
qdisc_pkt_len(skb));
if (err)
- return err;
+ return qdisc_drop(skb, sch);
}
err = qdisc_enqueue(skb, cl->qdisc);
--
2.7.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] qfq: don't leak skb if kzalloc fails
2016-06-08 12:31 [PATCH net] qfq: don't leak skb if kzalloc fails Florian Westphal
@ 2016-06-08 15:54 ` Eric Dumazet
2016-06-08 21:04 ` Florian Westphal
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2016-06-08 15:54 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
On Wed, 2016-06-08 at 14:31 +0200, Florian Westphal wrote:
> When we need to create a new aggregate to enqueue the skb we need
> to kzalloc the new struct.
>
> If that fails we returned ENOBUFS without freeing the skb.
>
> Spotted during code review.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/sched/sch_qfq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 8d2d8d9..435b970 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -1236,7 +1236,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> err = qfq_change_agg(sch, cl, cl->agg->class_weight,
> qdisc_pkt_len(skb));
> if (err)
> - return err;
> + return qdisc_drop(skb, sch);
> }
>
> err = qdisc_enqueue(skb, cl->qdisc);
Good catch, but it looks like you forgot :
cl->qstats.drops++;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] qfq: don't leak skb if kzalloc fails
2016-06-08 15:54 ` Eric Dumazet
@ 2016-06-08 21:04 ` Florian Westphal
0 siblings, 0 replies; 3+ messages in thread
From: Florian Westphal @ 2016-06-08 21:04 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2016-06-08 at 14:31 +0200, Florian Westphal wrote:
> > When we need to create a new aggregate to enqueue the skb we need
> > to kzalloc the new struct.
> >
> > If that fails we returned ENOBUFS without freeing the skb.
> >
> > Spotted during code review.
> >
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> > net/sched/sch_qfq.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> > index 8d2d8d9..435b970 100644
> > --- a/net/sched/sch_qfq.c
> > +++ b/net/sched/sch_qfq.c
> > @@ -1236,7 +1236,7 @@ static int qfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
> > err = qfq_change_agg(sch, cl, cl->agg->class_weight,
> > qdisc_pkt_len(skb));
> > if (err)
> > - return err;
> > + return qdisc_drop(skb, sch);
> > }
> >
> > err = qdisc_enqueue(skb, cl->qdisc);
>
> Good catch, but it looks like you forgot :
>
> cl->qstats.drops++;
Thanks, we have too many stats :-/
I'll send a v2, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-08 21:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-08 12:31 [PATCH net] qfq: don't leak skb if kzalloc fails Florian Westphal
2016-06-08 15:54 ` Eric Dumazet
2016-06-08 21:04 ` Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).