Re: [PATCH net] net_sched: fix skb memory leak in deferred qdisc drops

Eric Dumazet
2026-04-08T08:54:48Z

On Tue, Apr 7, 2026 at 10:14 AM Fernando Fernandez Mancera
<fmancera@suse.de> wrote:
>
>
>
> On 4/7/26 7:02 PM, Fernando Fernandez Mancera wrote:
> > When the network stack cleans up the deferred list via qdisc_run_end(),
> > it operates on the root qdisc. If the root qdisc do not implement the
> > TCQ_F_DEQUEUE_DROPS flag the packets queue to free are never freed and
> > gets stranded on the child's local to_free list.
> >
> > Fix this by making qdisc_dequeue_drop() aware of the root qdisc. It
> > fetches the root qdisc and check for the TCQ_F_DEQUEUE_DROPS flag. If
> > the flag is present, the packet is appended directly to the root's
> > to_free list. Otherwise, drop it directly as it was done before the
> > optimization was implemented.
> >
> > Fixes: a6efc273ab82 ("net_sched: use qdisc_dequeue_drop() in cake, codel, fq_codel")
> > Reported-by: Damilola Bello <damilola@aterlo.com>
> > Closes: https://lore.kernel.org/netdev/CAPgFtOLaedBMU0f_BxV2bXftTJSmJr018Q5uozOo5vVo6b9tjw@mail.gmail.com/
> > Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
> > ---
> > include/net/sch_generic.h | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> > index c3d657359a3d..61ba54e909f2 100644
> > --- a/include/net/sch_generic.h
> > +++ b/include/net/sch_generic.h
> > @@ -1170,12 +1170,18 @@ static inline void tcf_kfree_skb_list(struct sk_buff *skb)
> > static inline void qdisc_dequeue_drop(struct Qdisc *q, struct sk_buff *skb,
> > enum skb_drop_reason reason)
> > {
> > + struct Qdisc *root = qdisc_root_sleeping(q);
> > +
> > DEBUG_NET_WARN_ON_ONCE(!(q->flags & TCQ_F_DEQUEUE_DROPS));
> > DEBUG_NET_WARN_ON_ONCE(q->flags & TCQ_F_NOLOCK);
> >
> > - tcf_set_drop_reason(skb, reason);
> > - skb->next = q->to_free;
> > - q->to_free = skb;
> > + if (root->flags & TCQ_F_DEQUEUE_DROPS) {
> > + tcf_set_drop_reason(skb, reason);
> > + skb->next = q->to_free;
> > + q->to_free = skb;
> > + } else {
> > + kfree_skb_reason(skb, reason);
> > + }
> > }
> >
>
> *sigh* this is missing the following diff - tested it but forgot to
> amend it. Anyway, Eric let me know what do you think, if you like this
> solution I can send the v2 appending the skb to the root's to_free list
> properly.
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 61ba54e909f2..0e4166319c02 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -1177,8 +1177,8 @@ static inline void qdisc_dequeue_drop(struct Qdisc
> *q, struct sk_buff *skb,
>
> if (root->flags & TCQ_F_DEQUEUE_DROPS) {
> tcf_set_drop_reason(skb, reason);
> - skb->next = q->to_free;
> - q->to_free = skb;
> + skb->next = root->to_free;
> + root->to_free = skb;
> } else {
> kfree_skb_reason(skb, reason);
> }
>

Sure, please also use rcu_read_lock() before calling qdisc_root_sleeping()