Skip to content

[LTS 9.2] nvme-tcp: fix potential memory corruption in nvme_tcp_recv_pdu() - #238

Merged
PlaidCat merged 1 commit into
ctrliq:ciqlts9_2from
pvts-mat:ciqlts9_2-CVE-2025-21927
May 2, 2025
Merged

[LTS 9.2] nvme-tcp: fix potential memory corruption in nvme_tcp_recv_pdu()#238
PlaidCat merged 1 commit into
ctrliq:ciqlts9_2from
pvts-mat:ciqlts9_2-CVE-2025-21927

Conversation

@pvts-mat

Copy link
Copy Markdown
Contributor

[LTS 9.2]
CVE-2025-21927
VULN-56028

Problem

https://www.cve.org/CVERecord?id=CVE-2025-21927

In the Linux kernel, the following vulnerability has been resolved: nvme-tcp: fix potential memory corruption in nvme_tcp_recv_pdu() nvme_tcp_recv_pdu() doesn't check the validity of the header length. When header digests are enabled, a target might send a packet with an invalid header length (e.g. 255), causing nvme_tcp_verify_hdgst() to access memory outside the allocated area and cause memory corruptions by overwriting it with the calculated digest. Fix this by rejecting packets with an unexpected header length.

Analysis and Solution (same as for ciqlts8_8)

Context

NVME (Non-Volatile Memory Express) is a communication protocol designed for accessing high-speed storage media, particularly solid-state drives (SSDs). NVMe over Fabrics (NVMe-oF) is an extension of the NVMe protocol that allows NVMe commands to be sent over a network fabric, enabling remote access to NVMe storage devices.

The "target" mentioned in CVE description is the host providing access to the local NVME device (the server). The host importing the remote NVME device is called simply a "host", or "initiator" (the client). The module implementing NVMe-oF on target's side is nvmet-tcp, on the initiator's side it's nvme-tcp - the subject of this patch.

Applicability

All the key options related to NVMe-oF, specifically CONFIG_NVME_TCP enabling the nvme-tcp module, are enabled in ciqlts9_2. Per .config file created from configs/kernel-x86_64-rhel.config:

#
# NVME Support
#
CONFIG_NVME_COMMON=m
CONFIG_NVME_CORE=m
CONFIG_BLK_DEV_NVME=m
CONFIG_NVME_MULTIPATH=y
CONFIG_NVME_VERBOSE_ERRORS=y
# CONFIG_NVME_HWMON is not set
CONFIG_NVME_FABRICS=m
CONFIG_NVME_RDMA=m
CONFIG_NVME_FC=m
CONFIG_NVME_TCP=m
CONFIG_NVME_AUTH=y
CONFIG_NVME_TARGET=m
# CONFIG_NVME_TARGET_PASSTHRU is not set
CONFIG_NVME_TARGET_LOOP=m
CONFIG_NVME_TARGET_RDMA=m
CONFIG_NVME_TARGET_FC=m
CONFIG_NVME_TARGET_FCLOOP=m
CONFIG_NVME_TARGET_TCP=m
CONFIG_NVME_TARGET_AUTH=y
# end of NVME Support

Solution

The solution in the mainline kernel is provided in the ad95bab commit. It was not backported to any stable kernel older than 6.12.

Naive cherry-picking results in conflicts with git's attempt to introduce additional functions (nvme_tcp_tls_configured, nvme_tcp_queue_tls) and code branches (nvme_tcp_c2h_term packet type check in the nvme_tcp_recv_pdu function) introduced in more recent versions of the module but not related to the bug fix.

A small change was made to the nvme_tcp_recv_pdu_supported function introduced in the official fix ad95bab for the sake of nvme_tcp_recv_pdu's behavior consistency between the scenarios of receiving a packet with a proper and an improper header - the removal of the nvme_tcp_c2h_term case.

Consider the behavior cases in case a packet with a proper header was received:

Packet type:X ∈ {c2h_term}X ∈ {c2h_data, rsp, r2t}X ∉ {c2h_term, c2h_data, rsp, r2t}
aMainline, after patch (ad95bab)nvme_tcp_handle_Xnvme_tcp_handle_X"unsupported pdu type …", -EINVAL
bciqlts9_2, after patch, c2h_term included"unsupported pdu type …", -EINVALnvme_tcp_handle_X"unsupported pdu type …", -EINVAL
cciqlts9_2, after patch, c2h_term excluded"unsupported pdu type …", -EINVALnvme_tcp_handle_X"unsupported pdu type …", -EINVAL

Then in case a packet with an improper header was received:

Packet type:X ∈ {c2h_term}X ∈ {c2h_data, rsp, r2t}X ∉ {c2h_term, c2h_data, rsp, r2t}
xMainline, after patch (ad95bab)"pdu type %d has unexpected header length", -EPROTO"pdu type %d has unexpected header length", -EPROTO"unsupported pdu type …", -EINVAL
yciqlts9_2, after patch, c2h_term included"pdu type %d has unexpected header length", -EPROTO"pdu type %d has unexpected header length", -EPROTO"unsupported pdu type …", -EINVAL
zciqlts9_2, after patch, c2h_term excluded"unsupported pdu type …", -EINVAL"pdu type %d has unexpected header length", -EPROTO"unsupported pdu type …", -EINVAL

Solution a is to xnot as b is to y, but as c is to z, thus the c, z pair was chosen.

kABI check: passed

DEBUG=1 RELAXED_DEPS=1 CVE=CVE-2025-21927 ./ninja.sh _kabi_checked__x86_64--test--ciqlts9_2-CVE-2025-21927
[0/1] Check ABI of kernel [ciqlts9_2-CVE-2025-21927]
++ uname -m
+ python3 /data/src/ctrliq-github/kernel-dist-git-el-9.2/SOURCES/check-kabi -k /data/src/ctrliq-github/kernel-dist-git-el-9.2/SOURCES/Module.kabi_x86_64 -s vms/x86_64--build--ciqlts9_2/build_files/kernel-src-tree-ciqlts9_2-CVE-2025-21927/Module.symvers
kABI check passed
+ touch state/kernels/ciqlts9_2-CVE-2025-21927/x86_64/kabi_checked

Boot test: passed

boot-test.log

Kselftests: passed relative

Methodology

The selftests were source-compiled from the recent ciqlts9_2 branch (commit f10433c). The bpf suite was run from the kernel-selftests-internal package.

The tests were run using an explicit list which omitted certain tests known to give inconsistent results. Details in the src/run-kselftests.sh script of the rocky-patching project.

Coverage

bpf (except test_kmod.sh, test_progs, test_progs-no_alu32, test_sockmap), breakpoints, capabilities, cgroup (except test_memcontrol), clone3, core, cpu-hotplug, cpufreq, drivers/dma-buf, drivers/net/bonding, drivers/net/team, efivarfs, filesystems/binderfs, filesystems/epoll, firmware, fpu, ftrace, futex, gpio, intel_pstate, ipc, ir, kcmp, kexec, kvm, landlock, lib, livepatch, membarrier, memfd, memory-hotplug, mincore, mount, mqueue, nci, net (except reuseaddr_conflict, udpgso_bench.sh), net/forwarding (except sch_ets.sh, sch_red.sh, sch_tbf_ets.sh, sch_tbf_prio.sh, sch_tbf_root.sh, tc_police.sh), net/mptcp, netfilter (except nft_trans_stress.sh), nsfs, openat2, pid_namespace, pidfd, proc (except proc-pid-vm), pstore, ptrace, rlimits, rseq, rtc, seccomp, sgx, sigaltstack, size, splice, static_keys, sync, syscall_user_dispatch, sysctl, tc-testing, tdx, timens, timers (except raw_skew), tmpfs, tpm2, user, vDSO, vm, x86, zram

The coverage for the patch test was a bit narrowed to get rid of inconsequntial results.

Reference

kselftests–ciqlts9_2–run1.log
kselftests–ciqlts9_2–run2.log
kselftests–ciqlts9_2–run3.log
kselftests–ciqlts9_2–run4.log

Patch

kselftests–ciqlts9_2-CVE-2025-21927–run1.log

Comparison

ktests.xsh diff -d kselftests*.log
Column File
-------- ----------------------------------------------
Status0 kselftests--ciqlts9_2--run1.log
Status1 kselftests--ciqlts9_2--run2.log
Status2 kselftests--ciqlts9_2--run3.log
Status3 kselftests--ciqlts9_2--run4.log
Status4 kselftests--ciqlts9_2-CVE-2025-21927--run1.log
TestCase Status0 Status1 Status2 Status3 Status4 Summary
net/forwarding:dual_vxlan_bridge.sh fail pass fail fail diff
net:txtimestamp.sh pass pass fail pass diff
rtc:rtctest fail pass pass fail diff

All differences are contained within the reference kernel. The test run for the patched kernel was done in a different batch, with the tests observed to be nondeterministic removed, including those which showed different results for the ciqlts9_2 reference batch, thus the blank fields in the Status4 column.

Specific tests: suspended

See the situation for #234.

jira VULN-56028
cve CVE-2025-21927
commit-author Maurizio Lombardi <mlombard@redhat.com>
commit ad95bab
upstream-diff Removed `nvme_tcp_c2h_term' case from
`nvme_tcp_recv_pdu_supported' for the sake of consistency of
`nvme_tcp_recv_pdu''s behavior relative to the upstream
version, between the cases of proper and improper
header. (What could be considered as "`c2h_term' type support"
started with 84e0090 commit,
not included in `ciqlts9_2''s history, so
`nvme_tcp_recv_pdu_supported' in `ciqlts9_2' shouldn't report
the `nvme_tcp_c2h_term' type as supported.)
nvme_tcp_recv_pdu() doesn't check the validity of the header length.
When header digests are enabled, a target might send a packet with an
invalid header length (e.g. 255), causing nvme_tcp_verify_hdgst()
to access memory outside the allocated area and cause memory corruptions
by overwriting it with the calculated digest.
Fix this by rejecting packets with an unexpected header length.
Fixes: 3f2304f ("nvme-tcp: add NVMe over TCP host driver")
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Keith Busch <kbusch@kernel.org>
(cherry picked from commit ad95bab)
Signed-off-by: Marcin Wcisło <marcin.wcislo@conclusive.pl>

@PlaidCatPlaidCat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:shipit:

@bmastbergenbmastbergen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🥌

@PlaidCat
PlaidCat merged commit 326cfbf into ctrliq:ciqlts9_2May 2, 2025
bmastbergen pushed a commit to bmastbergen/kernel-src-tree that referenced this pull request Aug 29, 2025
jira LE-1907
Rebuild_History Non-Buildable kernel-5.14.0-427.18.1.el9_4
commit-author Daniel Borkmann <daniel@iogearbox.net>
commit cd13c91
Add a big batch of test coverage to assert all aspects of the tcx opts
attach, detach and query API:
# ./vmtest.sh -- ./test_progs -t tc_opts
[...]
ctrliq#238 tc_opts_after:OK
ctrliq#239 tc_opts_append:OK
ctrliq#240 tc_opts_basic:OK
ctrliq#241 tc_opts_before:OK
ctrliq#242 tc_opts_chain_classic:OK
ctrliq#243 tc_opts_demixed:OK
ctrliq#244 tc_opts_detach:OK
ctrliq#245 tc_opts_detach_after:OK
ctrliq#246 tc_opts_detach_before:OK
ctrliq#247 tc_opts_dev_cleanup:OK
ctrliq#248 tc_opts_invalid:OK
ctrliq#249 tc_opts_mixed:OK
ctrliq#250 tc_opts_prepend:OK
ctrliq#251 tc_opts_replace:OK
ctrliq#252 tc_opts_revision:OK
Summary: 15/0 PASSED, 0 SKIPPED, 0 FAILED
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/r/20230719140858.13224-8-daniel@iogearbox.net
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
(cherry picked from commit cd13c91)
Signed-off-by: Jonathan Maple <jmaple@ciq.com>
bmastbergen pushed a commit to bmastbergen/kernel-src-tree that referenced this pull request Aug 29, 2025
jira LE-1907
Rebuild_History Non-Buildable kernel-5.14.0-427.18.1.el9_4
commit-author Daniel Borkmann <daniel@iogearbox.net>
commit ccd9a8b
Add several new tcx test cases to improve test coverage. This also includes
a few new tests with ingress instead of clsact qdisc, to cover the fix from
commit dc644b5 ("tcx: Fix splat in ingress_destroy upon tcx_entry_free").
# ./test_progs -t tc
[...]
ctrliq#234 tc_links_after:OK
ctrliq#235 tc_links_append:OK
ctrliq#236 tc_links_basic:OK
ctrliq#237 tc_links_before:OK
ctrliq#238 tc_links_chain_classic:OK
ctrliq#239 tc_links_chain_mixed:OK
ctrliq#240 tc_links_dev_cleanup:OK
ctrliq#241 tc_links_dev_mixed:OK
ctrliq#242 tc_links_ingress:OK
ctrliq#243 tc_links_invalid:OK
ctrliq#244 tc_links_prepend:OK
ctrliq#245 tc_links_replace:OK
ctrliq#246 tc_links_revision:OK
ctrliq#247 tc_opts_after:OK
ctrliq#248 tc_opts_append:OK
ctrliq#249 tc_opts_basic:OK
ctrliq#250 tc_opts_before:OK
ctrliq#251 tc_opts_chain_classic:OK
ctrliq#252 tc_opts_chain_mixed:OK
ctrliq#253 tc_opts_delete_empty:OK
ctrliq#254 tc_opts_demixed:OK
ctrliq#255 tc_opts_detach:OK
ctrliq#256 tc_opts_detach_after:OK
ctrliq#257 tc_opts_detach_before:OK
ctrliq#258 tc_opts_dev_cleanup:OK
ctrliq#259 tc_opts_invalid:OK
ctrliq#260 tc_opts_mixed:OK
ctrliq#261 tc_opts_prepend:OK
ctrliq#262 tc_opts_replace:OK
ctrliq#263 tc_opts_revision:OK
[...]
Summary: 44/38 PASSED, 0 SKIPPED, 0 FAILED
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/r/8699efc284b75ccdc51ddf7062fa2370330dc6c0.1692029283.git.daniel@iogearbox.net
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
(cherry picked from commit ccd9a8b)
Signed-off-by: Jonathan Maple <jmaple@ciq.com>
Sign up for freeto 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

@pvts-mat@PlaidCat@bmastbergen