Skip to content

[LTS 8.8] net: tls, update curr on splice as well #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: ciqlts8_8
Choose a base branch
from

Conversation

pvts-mat
Copy link
Contributor

@pvts-mat pvts-mat commented Jun 5, 2025

[LTS 8.8]
CVE-2024-0646
VULN-6841

Problem

https://access.redhat.com/security/cve/CVE-2024-0646

An out-of-bounds memory write flaw was found in the Linux kernel’s Transport Layer Security functionality in how a user calls a function splice with a ktls socket as the destination. This flaw allows a local user to crash or potentially escalate their privileges on the system.

Applicability analysis

The tls module is enabled in ciqlts8_8:

grep 'CONFIG_TLS=' configs/kernel-{x86_64,aarch64,ppc64le,s390x}.config

configs/kernel-x86_64.config:CONFIG_TLS=m
configs/kernel-aarch64.config:CONFIG_TLS=m
configs/kernel-ppc64le.config:CONFIG_TLS=m
configs/kernel-s390x.config:CONFIG_TLS=m

The commit referenced in the mainline fix c5a5950 as introducing the bug is d829e9c. It's present in the history of LTS 9.4 and upstream stable Linux 5.15 where the fix was backported in 8ad16a7 and ba5efd8, respectively. Similar situation for the recently merged PR #305 for LTS 9.2.

The referenced commit d829e9c is not present in official stable Linux 4.18, which is most probably the reason the fix wasn't backported to any stable relase older than 5.4. However, it can be found in the "Rebuild_History BUILDABLE"-type commit e19ec64 of Rocky LTS 8.8 (see ciq/ciq_backports/kernel-4.18.0-147.el8/d829e9c4.failed).

Based on this it was concluded that the vulnerability applies to ciqlts8_8.

Solution

The solution for ciqlts8_8 is the same as in ciqlts9_2, because the function tls_sw_do_sendpage modified in ciqlts9_2 is exactly the same as in ciqlts8_8. For the explanation of the fix in ciqlts9_2 please see #305.

kABI check: passed

DEBUG=1 CVE=CVE-2024-0646 ./ninja.sh _kabi_checked__x86_64--test--ciqlts8_8-CVE-2024-0646

[0/1] Check ABI of kernel [ciqlts8_8-CVE-2024-0646]
++ 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-2024-0646/Module.symvers
kABI check passed
+ touch state/kernels/ciqlts8_8-CVE-2024-0646/x86_64/kabi_checked

Boot test: passed

boot-test.log

Kselftests

Coverage

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

Reference

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

Patch

kselftests–ciqlts8_8-CVE-2024-0646–run1.log
kselftests–ciqlts8_8-CVE-2024-0646–run2.log

Comparison

The reference and patched kernel 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-CVE-2024-0646--run1.log
Status3   kselftests--ciqlts8_8-CVE-2024-0646--run2.log

In particular the net:tls test is passing in both versions, with the same logs:

$ ktests.xsh show_groups --test net:tls -s kselftests*.log

kselftests--ciqlts8_8--run1.log:
kselftests--ciqlts8_8--run2.log:
kselftests--ciqlts8_8-CVE-2024-0646--run1.log:
kselftests--ciqlts8_8-CVE-2024-0646--run2.log:
net:tls:
# TAP version 13
# 1..91
# # Starting 91 tests from 4 test cases.
# #  RUN           global.non_established ...
# #            OK  global.non_established
# ok 1 global.non_established
# #  RUN           global.keysizes ...
# #            OK  global.keysizes
…
# ok 91 tls.13.shutdown_reuse
# # PASSED: 91 / 91 tests passed.
# # Totals: pass:91 fail:0 xfail:0 xpass:0 skip:0 error:0
ok 1 selftests: net: tls

Specific tests: skipped

Appendix

Below is an improved version of a similar table given in the appendix of #305, but with "Rebuild_History BUILDABLE"-type commits included, which left no commit from the net/tls/tls_sw.c history for LTS 8.6, 8.8, 9.2, 9.4 not cross-referenced.

$ cve-research/git-analysis.xsh histories -C …/kernel-src-tree \
  kernel-mainline linux-5.15.y linux-4.19.y ciqlts9_4 ciqlts9_2 ciqlts8_8 ciqlts8_6 \
  --file net/tls/tls_sw.c \
  --keys sha subject version \
  --ref-opts='--no-merges' --format-main='%h %cd %s' --format-other='%h %cd'

tls_sw-history.txt

Symbols:

  • =: exact same commit,
  • ~: cherry-picked backport,
  • #: rebuild-type bulk commit with all the cherry-picks which failed to be ~.

jira VULN-6841
cve CVE-2024-0646
commit-author John Fastabend <[email protected]>
commit c5a5950
upstream-diff used linux-stable LT-5.15 sha ba5efd8

commit c5a5950 upstream.

The curr pointer must also be updated on the splice similar to how
we do this for other copy types.

Fixes: d829e9c ("tls: convert to generic sk_msg interface")
	Signed-off-by: John Fastabend <[email protected]>
	Reported-by: Jann Horn <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
	Signed-off-by: Jakub Kicinski <[email protected]>
	Signed-off-by: Greg Kroah-Hartman <[email protected]>
(cherry picked from commit ba5efd8)
	Signed-off-by: Marcin Wcisło <[email protected]>
@PlaidCat
Copy link
Collaborator

PlaidCat commented Jun 6, 2025

Yeah the "fixes" is buried deep in the origin of Rocky8

[jmaple@devbox kernel-src-tree]$ git log --grep d829e9c --oneline

3b1ba85a6129 bpf: Fix running sk_skb program types with ktls
9e2630cf4167 bpf: Refactor sockmap redirect code so its easy to reuse
fa94d09f0d6e net/tls: remove the dead inplace_crypto code
ac2cab3deeaf net/tls: fix sk_msg trim on fallback to copy mode
c0e95b53c66e net: tls, correctly account for copied bytes with multiple sk_msgs
30544fc57cc1 Prevent overflow of sk_msg in sk_msg_clone()
69d65b1be1d4 tls: Do not call sk_memcopy_from_iter with zero length
fe773b563af7 bpf: sk_msg, fix socket data_ready events
eff830d6d3b6 tls: convert to generic sk_msg interface

eff830d

I also stash all the failed backports:

[jmaple@devbox kernel-src-tree]$ find ciq/ciq_backports/ | xargs grep "d829e9c"  2>/dev/null
ciq/ciq_backports/kernel-4.18.0-147.el8/552de910.failed:Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface")
ciq/ciq_backports/kernel-4.18.0-147.el8/5c1e7e94.failed:Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface")
ciq/ciq_backports/kernel-4.18.0-147.el8/648ee6ce.failed:Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface")
ciq/ciq_backports/kernel-4.18.0-147.el8/65a10e28.failed:Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface")
ciq/ciq_backports/kernel-4.18.0-147.el8/d829e9c4.failed:commit d829e9c4112b52f4f00195900fd4c685f61365ab
ciq/ciq_backports/kernel-4.18.0-147.el8/d829e9c4.failed:ciq/ciq_backports/kernel-4.18.0-147.el8/d829e9c4.failed
ciq/ciq_backports/kernel-4.18.0-147.el8/d829e9c4.failed:(cherry picked from commit d829e9c4112b52f4f00195900fd4c685f61365ab)
ciq/ciq_backports/kernel-4.18.0-147.el8/rebuild.details.txt:d829e9c4112b52f4f00195900fd4c685f61365ab tls: convert to generic sk_msg interface

Its not been super useful yet(and may not continue in the future) but it comes from this rebuild which contained 12,000 commits and only could cleanly pick 86%
https://github.com/ctrliq/kernel-src-tree/blob/rocky8_8/ciq/ciq_backports/kernel-4.18.0-147.el8/rebuild.details.txt

The above grep is why we make empty commits for failed backports so hopefully we can git log --grep <sha> of the upstream commit to find it.

There is a bit of a learning curve with this kernel setup but the other options is we just have the big splats from the REEBUILD type and then the engineer has to part potentially thousands of changes from a changelog associated with the RPM we got the tarball from. I did this back in the day at 2.6.32 and 3.0 era and it made me disdain the blob tarball method from Enterprise Linuxes, if there is no tree provided, this is the best we can do with the resources and time I had at the time.

Copy link
Collaborator

@PlaidCat PlaidCat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@pvts-mat
Copy link
Contributor Author

pvts-mat commented Jun 6, 2025

Its not been super useful yet(and may not continue in the future) but it comes from this rebuild which contained 12,000 commits and only could cleanly pick 86%
https://github.com/ctrliq/kernel-src-tree/blob/rocky8_8/ciq/ciq_backports/kernel-4.18.0-147.el8/rebuild.details.txt

Yes, that's where I got this ciq/ciq_backports/kernel-4.18.0-147.el8/d829e9c4.failed from. This tool I used in the "Appendix" section is aware of those rebuild.details.txt files and searches them to cross-reference with the upstream commits, so you get a continuous sequence corresponding to the upstream, with lumps marked with #, like

kernel-mainline                                                                            ciqlts8_6
-----------------------------------------------------------------------------------------  -----------
...
00e237074 iov_iter: Use accessor function                                                  # 9c3767b38
02c558b2d bpf: sockmap, support for msg_peek in sk_msg with redirect ingress               # e19ec644e
d3b18ad31 tls: add bpf support to sk_msg handling                                          # e19ec644e
924ad65ed tls: replace poll implementation with read hook                                  ~ 62e042e64
d829e9c41 tls: convert to generic sk_msg interface                                         # e19ec644e
4e6d47206 tls: Add support for inplace records encryption                                  ~ fb99c8d53
80ece6a03 tls: Remove redundant vars from tls record structure                             ~ 40a284a2e
...

One thing with empty commits to be aware of is that they disappear when logging a specific file's history, like git log -- some/file.c.

It is helpful, this backengineered history, definitely better to have 14% out of 12k to be lumped into one ommit than 100%. Good job 👍. The blobs are done for a reason, and this reason is exactly to make the life of the engineer working with this source code harder 😅

Coming back to the PR, I wasn't really convinced that it was d829e9c which actually introduced the bug, but I guess I will have to trust John Fasteband on this.

Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥌

@PlaidCat
Copy link
Collaborator

PlaidCat commented Jun 6, 2025

The blobs are done for a reason, and this reason is exactly to make the life of the engineer working with this source code harder 😅

Coming back to the PR, I wasn't really convinced that it was d829e9c which actually introduced the bug, but I guess I will have to trust John Fasteband on this.

Yeah, you can only be an expert in so much when you have a start up sized operation or are being a kernel generalist so you trust and verify as much as you can.

Thanks for diving and learning the fun world of EL Kernel ABI stability and what that does to the kernel versus upstream its usually extremely unsettling the first big thing you run into and when it happens again you're kinda prepared with a "sigh" and know it will be a while before you're done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants