[LTS 8.8] net/sched: sch_qfq: account for stab overhead in qfq_enqueue

[pvts-mat]

[LTS 8.8]
CVE-2023-3611
VULN-6560

Problem

https://www.cve.org/CVERecord?id=CVE-2023-3611

An out-of-bounds write vulnerability in the Linux kernel's net/sched: sch_qfq component can be exploited to achieve local privilege escalation. The qfq_change_agg() function in net/sched/sch_qfq.c allows an out-of-bounds write because lmax is updated according to packet sizes without bounds checks.

Applicability

The vulnerability applies to the sch_qfq module which is enabled in ciqlts8_8:

grep CONFIG_NET_SCH_QFQ= configs/*

configs/kernel-aarch64-debug.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-aarch64.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-ppc64le-debug.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-ppc64le.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-s390x-debug.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-s390x-zfcpdump.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-s390x.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-x86_64-debug.config:CONFIG_NET_SCH_QFQ=m
configs/kernel-x86_64.config:CONFIG_NET_SCH_QFQ=m

Solution

The mainline fix is provided in the commit 3e33708. The official backport to Linux 4.19 (closest to ciqlts8_8's 4.18) is given in the commit ee3bc82. Note that it's different from the mainline fix, as it doesn't use the introduced in the meantime (6f22c11) QFQ_MAX_LMAX constant, inlining its definition instead

3e33708:

net/sched/sch_qfq.c:116:

#define QFQ_MAX_LMAX		(1UL << QFQ_MTU_SHIFT)

net/sched/sch_qfq.c:387:

if (lmax > QFQ_MAX_LMAX)
	return -EINVAL;

ee3bc82:

net/sched/sch_qfq.c:393:

if (lmax > (1UL << QFQ_MTU_SHIFT))
	return -EINVAL;

The already applied fix for Rocky LTS 8.6 gets around it by first picking 2536989 (as 6f22c11) which introduces QFQ_MAX_LMAX and then the mainline fix 3e33708 (as fe72210):

git log --oneline -n 2 fe72210071638a9c4cedfa4a09629cf284fd9631

fe7221007 net/sched: sch_qfq: account for stab overhead in qfq_enqueue
6f22c114d net/sched: sch_qfq: refactor parsing of netlink parameters

git log -n 1 fe72210071638a9c4cedfa4a09629cf284fd9631 | grep -A 1 upstream-diff

upstream-diff Cherry-pick is clean however QFQ_MAX_LMAX is undeclared so
a prereq commit was needed

For ciqlts8_8's fix the stable's branch backport ee3bc82 was used directly as it applies without any conflicts or code changes and provides a simpler solution while remaining no less official than the mainlne fix.

kABI check: passed

DEBUG=1 CVE=CVE-2023-3611 ./ninja.sh _kabi_checked__$(uname -m)--test--ciqlts8_8-CVE-2023-3611

[1/2] Check ABI of kernel [ciqlts8_8-CVE-2023-3611]
++ uname -m
+ python3 /data/src/ctrliq-github/kernel-dist-git-el-8.8/SOURCES/check-kabi -k /data/src/ctrliq-github/kernel-dist-git-el-8.8/SOURCES/Module.kabi_x86_64 -s vms/x86_64--build--ciqlts8_8/build_files/kernel-src-tree-ciqlts8_8-CVE-2023-3611/Module.symvers
kABI check passed
+ touch state/kernels/ciqlts8_8-CVE-2023-3611/x86_64/kabi_checked

Boot test: passed

boot-test.log

Kselftests: passed relative

Coverage

Specific tests were skipped which proved to be unreliable in the past.

android, bpf (except test_progs, test_xsk.sh, test_progs-no_alu32, test_kmod.sh, test_sockmap), breakpoints, capabilities, cgroup, core, cpu-hotplug, cpufreq, drivers/net/bonding, drivers/net/team, efivarfs, exec, firmware, fpu, ftrace, futex, gpio, intel_pstate, ipc, kcmp, kexec, kvm, lib, livepatch, membarrier, memfd, memory-hotplug, mount, mqueue, net/forwarding (except mirror_gre_vlan_bridge_1q.sh, sch_ets.sh, ipip_hier_gre_keys.sh, sch_tbf_ets.sh, tc_actions.sh, mirror_gre_bridge_1d_vlan.sh, sch_tbf_prio.sh, sch_tbf_root.sh), net/mptcp (except simult_flows.sh), net (except xfrm_policy.sh, udpgro_fwd.sh, gro.sh, txtimestamp.sh, udpgso_bench.sh, ip_defrag.sh, reuseaddr_conflict, reuseport_addr_any.sh), netfilter (except nft_trans_stress.sh), nsfs, pstore, ptrace, rseq, sgx, sigaltstack, size, splice, static_keys, sync, sysctl, tc-testing, tdx, timens, timers (except raw_skew), tpm2, user, vm, x86, zram

Reference

kselftests–ciqlts8_8–run3.log
kselftests–ciqlts8_8–run2.log
kselftests–ciqlts8_8–run1.log

Patch

kselftests–ciqlts8_8-CVE-2023-3611–run3.log
kselftests–ciqlts8_8-CVE-2023-3611–run2.log
kselftests–ciqlts8_8-CVE-2023-3611–run1.log

Comparison

All test results are the same

./ktests.xsh diff -d kselftests*.log

Column    File
--------  ---------------------------------------------
Status0   kselftests--ciqlts8_8--run1.log
Status1   kselftests--ciqlts8_8--run2.log
Status2   kselftests--ciqlts8_8--run3.log
Status3   kselftests--ciqlts8_8-CVE-2023-3611--run1.log
Status4   kselftests--ciqlts8_8-CVE-2023-3611--run2.log
Status5   kselftests--ciqlts8_8-CVE-2023-3611--run3.log

Specific tests: skipped

[bmastbergen]

The change itself looks good to me, and the justification for pulling the 4.19 stable version is fine. The only thing I don't know is what we want from a book keeping perspective in the commit log. @PlaidCat will any of our scripts be confused by two different 'commit hash' lines in the commit log for a single commit?

[PlaidCat]

The official backport to Linux 4.19 (closest to ciqlts8_8's 4.18)

To clarify this is only true sometimes as this kernel 8.8 has ~100k commits from upstream in it from 4.18 -> 6.x (I don't remember which off the top of my head kernel.org was up to when 8.8 was to start) so this is not always a good evaluation, and the primary reason we've defaulted to the Linus mainline.

WRT to tooling: Right now we don't use anything that look at the GKH (stable) kernel tree when it comes to automation, so unless both trees are cloned (or you just use stable's tree) you won't find that sha. I know there is another header line from stable that uses the old method of defining the reference to Linus's mainline tree but our tooling stops looking at the first blank line.

I think I'd prefer it to be something like

jira VULN-6560
cve CVE-2023-3611
commit-author Pedro Tammela <[email protected]>
commit 3e337087c3b5805fe0b8a46ba622a962880b5d64
upstream-diff used linux-stable LT-4.19 sha ee3bc829f9b4df96d208d58b654e400fa1f3b46c

<cherry-pick from linux-stable>

While its called out in the PR its not obvious where the actual code change comes from inside the commit log, so to discover it was from the LT-4.19 you'd have to come to the commit in github and then follow the PR links. If we were to ever migrate from GitHub to other systems PRs could become lost or no longer accurate so context as close to the code (ie in code or commit message) is the only way to prevent loss of context.
We could probably deduce this is from GHK stable but the rational is lost if the above isn't done correctly.

Note the git migration (and ticket systems) loss of data has happened to me at multiple companies so its not a theoretical.
Additionally you can look at the 2.6 seed for the kernel itself where all changes before that are lost to time.

[bmastbergen]

jira VULN-6560
cve CVE-2023-3611
commit-author Pedro Tammela <[email protected]>
commit 3e337087c3b5805fe0b8a46ba622a962880b5d64
upstream-diff used linux-stable LT-4.19 sha ee3bc829f9b4df96d208d58b654e400fa1f3b46c

<cherry-pick from linux-stable>

I like this ^^^ 👍

[pvts-mat]

To clarify this is only true sometimes as this kernel 8.8 has ~100k commits from upstream in it from 4.18 -> 6.x (I don't remember which off the top of my head kernel.org was up to when 8.8 was to start) so this is not always a good evaluation, and the primary reason we've defaulted to the Linus mainline.

I just learned it clearly from the other PR #282

PRs could become lost or no longer accurate so context as close to the code (ie in code or commit message) is the only way to prevent loss of context.

Relying on GitHub PRs should definitely be avoided (and migrating from GH encouraged 👍)
I just thought we have all the official Linux trees to pick cherries from and deducing from GKH stable is assumed.

[PlaidCat]

Thanks for the change

[thefossguy-ciq]

🚤