[PATCH] net/sched: reset sch_plug epoch state

[PATCH] net/sched: reset sch_plug epoch state

[shaos]

Hello,

sch_plug's reset callback purges the skb queue but leaves the private
packet-epoch counters intact.  If a buffered epoch is reset by a link
reset, a later TCQ_PLUG_RELEASE_ONE can use the old pkts_last_epoch
count as release credit and transmit a fresh packet that was never part
of the completed epoch.

The reproducer confirms this after link down/up: four buffered frames
are dropped by reset, then one new frame is held until a single release
command delivers it.  Three close controls do not deliver the frame.
The same fixed-kernel test keeps it queued.  This breaks the output
commit guarantee documented for checkpoint/replication users of the
plug qdisc.  It requires CAP_NET_ADMIN to configure/reset the qdisc and
is a correctness issue; no memory corruption or privilege escalation
was observed.

HEAD commit:    c03c1a14507dfaf8b856c2608f9ac216021fc222
Git tree:       30fd2ef82912c26cf3a58ad0ba6afc375fa851ac
Source version: Linux 7.3
Compiler:       gcc 11.4.0, GNU ld 2.38

The console logs, full kernel config, PoC, and initramfs runner are
available from one fixed-revision Gist:

Console output:
https://gist.githubusercontent.com/shaos26/f514dff98be5c90da07bc3467214ef10/raw/e16e1fa984b297ef7bf18e2c8a5992fc63daa062/console-output.txt
Kernel config:
https://gist.githubusercontent.com/shaos26/f514dff98be5c90da07bc3467214ef10/raw/e16e1fa984b297ef7bf18e2c8a5992fc63daa062/kernel-config.txt
PoC source:
https://gist.githubusercontent.com/shaos26/f514dff98be5c90da07bc3467214ef10/raw/e16e1fa984b297ef7bf18e2c8a5992fc63daa062/poc.c
Initramfs init:
https://gist.githubusercontent.com/shaos26/f514dff98be5c90da07bc3467214ef10/raw/e16e1fa984b297ef7bf18e2c8a5992fc63daa062/initramfs-init
Runner:
https://gist.githubusercontent.com/shaos26/f514dff98be5c90da07bc3467214ef10/raw/e16e1fa984b297ef7bf18e2c8a5992fc63daa062/reproduce.sh

Clone the Gist and kernel source, build the kernel with the attached
config, and run the self-contained initramfs builder/runner:

    git clone \
      https://gist.github.com/shaos26/f514dff98be5c90da07bc3467214ef10.git \
      sch-plug-repro
    cd sch-plug-repro
    git clone \
      https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git \
      linux
    git -C linux checkout c03c1a14507dfaf8b856c2608f9ac216021fc222
    cp kernel-config.txt linux/.config
    make -C linux olddefconfig
    make -C linux -j"$(nproc)" bzImage
    KERNEL="$PWD/linux/arch/x86/boot/bzImage" \
      LOG=/tmp/sch-plug-positive.log sh ./reproduce.sh

The runner builds a static PoC and initramfs, then boots QEMU.  The PoC
creates a veth pair, attaches sch_plug, performs the reset/release
sequence, and records tagged frames.  It requires GCC, BusyBox, cpio,
gzip, and QEMU on the host.  The guest runs as root; the equivalent
direct trigger requires CAP_NET_ADMIN and CAP_NET_RAW.  The attached
console output contains two positive QEMU runs and patched-kernel
controls.

Fixes: fe6bea7f1f3a ("sch_plug: purge buffered packets during reset")
Signed-off-by: shaos <shawnq1@163.com>
---
 net/sched/sch_plug.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c
--- a/net/sched/sch_plug.c
+++ b/net/sched/sch_plug.c
@@ -202,6 +202,18 @@
 	return 0;
 }
 
+static void plug_reset(struct Qdisc *sch)
+{
+	struct plug_sched_data *q = qdisc_priv(sch);
+
+	qdisc_reset_queue(sch);
+	q->pkts_current_epoch = 0;
+	q->pkts_last_epoch = 0;
+	q->pkts_to_release = 0;
+	q->unplug_indefinite = false;
+	q->throttled = true;
+}
+
 static struct Qdisc_ops plug_qdisc_ops __read_mostly = {
 	.id          =       "plug",
 	.priv_size   =       sizeof(struct plug_sched_data),
@@ -210,7 +222,7 @@
 	.peek        =       qdisc_peek_dequeued,
 	.init        =       plug_init,
 	.change      =       plug_change,
-	.reset       =	     qdisc_reset_queue,
+	.reset       =	     plug_reset,
 	.owner       =       THIS_MODULE,
 };
 MODULE_ALIAS_NET_SCH("plug");
-- 
2.34.1

Re: [PATCH] net/sched: reset sch_plug epoch state

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.