[PATCH net] net/sched: sch_qfq: do not free existing class in qfq_change_class()

[PATCH net] net/sched: sch_qfq: do not free existing class in qfq_change_class()

[Eric Dumazet]

Fixes qfq_change_class() error case.

cl->qdisc and cl should only be freed if a new class and qdisc
were allocated, or we risk various UAF.

Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Reported-by: syzbot+07f3f38f723c335f106d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6965351d.050a0220.eaf7.00c5.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_qfq.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index f4013b547438ffe1bdc8ba519971a1681df4700b..9d59090bbe934ad56ab08a59708aab375aa77cf0 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -529,8 +529,10 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	return 0;
 
 destroy_class:
-	qdisc_put(cl->qdisc);
-	kfree(cl);
+	if (!existing) {
+		qdisc_put(cl->qdisc);
+		kfree(cl);
+	}
 	return err;
 }
 
-- 
2.52.0.457.g6b5491de43-goog

Re: [PATCH net] net/sched: sch_qfq: do not free existing class in qfq_change_class()

[Jamal Hadi Salim]

On Mon, Jan 12, 2026 at 12:56 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Fixes qfq_change_class() error case.
>
> cl->qdisc and cl should only be freed if a new class and qdisc
> were allocated, or we risk various UAF.
>
> Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> Reported-by: syzbot+07f3f38f723c335f106d@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/6965351d.050a0220.eaf7.00c5.GAE@google.com/T/#u
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Eric,
The patch looks correct.  Initially i was scratching my head trying to
see how you knew it was the same issue. I gues on a UAF even without a
repro syzbot can tell you exactly where the UAF happened?

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

>  net/sched/sch_qfq.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index f4013b547438ffe1bdc8ba519971a1681df4700b..9d59090bbe934ad56ab08a59708aab375aa77cf0 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -529,8 +529,10 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
>         return 0;
>
>  destroy_class:
> -       qdisc_put(cl->qdisc);
> -       kfree(cl);
> +       if (!existing) {
> +               qdisc_put(cl->qdisc);
> +               kfree(cl);
> +       }
>         return err;
>  }
>
> --
> 2.52.0.457.g6b5491de43-goog
>

Re: [PATCH net] net/sched: sch_qfq: do not free existing class in qfq_change_class()

[Eric Dumazet]

On Mon, Jan 12, 2026 at 7:27 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Mon, Jan 12, 2026 at 12:56 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > Fixes qfq_change_class() error case.
> >
> > cl->qdisc and cl should only be freed if a new class and qdisc
> > were allocated, or we risk various UAF.
> >
> > Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> > Reported-by: syzbot+07f3f38f723c335f106d@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/netdev/6965351d.050a0220.eaf7.00c5.GAE@google.com/T/#u
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Eric,
> The patch looks correct.  Initially i was scratching my head trying to
> see how you knew it was the same issue. I gues on a UAF even without a
> repro syzbot can tell you exactly where the UAF happened?

Without a repro, I was still able to root-cause the issue to the
unexpected kfree().

syzbot report was sent as-is on the mailing list, I had no more information.

Thanks.

>
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> cheers,
> jamal
>
> >  net/sched/sch_qfq.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> > index f4013b547438ffe1bdc8ba519971a1681df4700b..9d59090bbe934ad56ab08a59708aab375aa77cf0 100644
> > --- a/net/sched/sch_qfq.c
> > +++ b/net/sched/sch_qfq.c
> > @@ -529,8 +529,10 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
> >         return 0;
> >
> >  destroy_class:
> > -       qdisc_put(cl->qdisc);
> > -       kfree(cl);
> > +       if (!existing) {
> > +               qdisc_put(cl->qdisc);
> > +               kfree(cl);
> > +       }
> >         return err;
> >  }
> >
> > --
> > 2.52.0.457.g6b5491de43-goog
> >

Re: [PATCH net] net/sched: sch_qfq: do not free existing class in qfq_change_class()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 12 Jan 2026 17:56:56 +0000 you wrote:
> Fixes qfq_change_class() error case.
> 
> cl->qdisc and cl should only be freed if a new class and qdisc
> were allocated, or we risk various UAF.
> 
> Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> Reported-by: syzbot+07f3f38f723c335f106d@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/6965351d.050a0220.eaf7.00c5.GAE@google.com/T/#u
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> [...]

Here is the summary with links:
  - [net] net/sched: sch_qfq: do not free existing class in qfq_change_class()
    https://git.kernel.org/netdev/net/c/3879cffd9d07

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html