Skip to content

[fips-legacy-8] net/ulp: prevent ULP without clone op from entering the LISTEN status #393

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

Conversation

bmastbergen
Copy link
Collaborator

jira VULN-8789
cve CVE-2023-0461

First I'll say @pvts-mat did a great write-up on this CVE in this PR #282 so you may want to read that if what I say below doesn't make sense. I'm going to try to give the abbreviated version below....

This CVE was fixed upstream with commit 2c02d41. And later, there was an upstream fix for that commit in commit 8ccc993. Those are the basis for the two commits in this PR. But the backport of commit 2c02d41 requires a change from upstream in order to work correctly on some older kernels. It was actually improperly backported to some stable kernels, and then had to be fixed in another unique, non-upstream based commit. An example of that is LT 5.10 commit fdaf885. Since that commit (or one like it) doesn't exists in the upstream, I chose to incorporate that change into my backport of commit 2c02d41 right off the bat (as described in upstream-diff tag of the commit message). Think of it as the change as it should have originally been backported to stable kernels. Then the upstream fix commit 8ccc993 applied cleanly on top of it. In ciqlts8_6 we actually took the improper backport as it was originally done in stable kernels, and then @pvts-mat fixed that along with 8ccc993 in PR 282. Although the commits to get there are different, we will end up with the same code in fips-legacy-8 and ciqlts8_6. I hope that makes sense.

Build Log

/home/brett/kernel-src-tree
  CLEAN   scripts/basic
  CLEAN   scripts/kconfig
  CLEAN   include/config include/generated arch/x86/include/generated
  CLEAN   .config .config.old
[TIMER]{MRPROPER}: 13s
x86_64 architecture detected, copying config
'configs/kernel-x86_64.config' -> '.config'
Setting Local Version for build
CONFIG_LOCALVERSION="-b_f-l-8-c_4.18.0-425.13.1_VULN-8789-25bb22c30da5"
Making olddefconfig
--
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf  --olddefconfig Kconfig
#
# configuration written to .config
#
Starting Build
scripts/kconfig/conf  --syncconfig Kconfig
  SYSTBL  arch/x86/include/generated/asm/syscalls_32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_32_ia32.h
  SYSTBL  arch/x86/include/generated/asm/syscalls_64.h
  SYSHDR  arch/x86/include/generated/asm/unistd_64_x32.h
--
  LD [M]  sound/usb/usx2y/snd-usb-usx2y.ko
  LD [M]  sound/virtio/virtio_snd.ko
  LD [M]  sound/x86/snd-hdmi-lpe-audio.ko
  LD [M]  sound/xen/snd_xen_front.ko
  LD [M]  virt/lib/irqbypass.ko
[TIMER]{BUILD}: 1000s
Making Modules
  INSTALL arch/x86/crypto/blowfish-x86_64.ko
  INSTALL arch/x86/crypto/camellia-aesni-avx-x86_64.ko
  INSTALL arch/x86/crypto/camellia-aesni-avx2.ko
  INSTALL arch/x86/crypto/camellia-x86_64.ko
--
  INSTALL sound/virtio/virtio_snd.ko
  INSTALL sound/x86/snd-hdmi-lpe-audio.ko
  INSTALL sound/xen/snd_xen_front.ko
  INSTALL virt/lib/irqbypass.ko
  DEPMOD  4.18.0-b_f-l-8-c_4.18.0-425.13.1_VULN-8789-25bb22c30da5+
[TIMER]{MODULES}: 13s
Making Install
sh ./arch/x86/boot/install.sh 4.18.0-b_f-l-8-c_4.18.0-425.13.1_VULN-8789-25bb22c30da5+ arch/x86/boot/bzImage \
	System.map "/boot"
[TIMER]{INSTALL}: 104s
Checking kABI
Checking kABI
kABI check passed
Setting Default Kernel to /boot/vmlinuz-4.18.0-b_f-l-8-c_4.18.0-425.13.1_VULN-8789-25bb22c30da5+ and Index to 0
Hopefully Grub2.0 took everything ... rebooting after time metrices
[TIMER]{MRPROPER}: 13s
[TIMER]{BUILD}: 1000s
[TIMER]{MODULES}: 13s
[TIMER]{INSTALL}: 104s
[TIMER]{TOTAL} 1146s
Rebooting in 10 seconds

Testing

selftest-4.18.0-425.13.1.el8.ciqfipscompliant.40.1.x86_64.log

selftest-4.18.0-b_f-l-8-c_4.18.0-425.13.1_VULN-8789-25bb22c30da5+.log

brett@lycia ~/ciq/vuln-8789 % grep ^ok selftest-4.18.0-425.13.1.el8.ciqfipscompliant.40.1.x86_64.log | wc -l
230
brett@lycia ~/ciq/vuln-8789 % grep ^ok selftest-4.18.0-b_f-l-8-c_4.18.0-425.13.1_VULN-8789-25bb22c30da5+.log | wc -l
230
brett@lycia ~/ciq/vuln-8789 %

jira VULN-8789
cve CVE-2023-0461
commit-author Paolo Abeni <[email protected]>
commit 2c02d41
upstream-diff In inet_csk_listen_start ret is reinitialized to
              -EADDRINUSE after the new inet_ulp_can_listen check.
              The upstream change did not do this because err would
              inevitably be set later in the function in that version.
              In this kernel there is no future inevitable set of err
              so we need to set it back to -EADDRINUSE to ensure this
              function will return -EADDERINUSE if ->get_port() fails.
              See LT 5.10 commit fdaf885
              for the inspiration for this upstream-diff

When an ULP-enabled socket enters the LISTEN status, the listener ULP data
pointer is copied inside the child/accepted sockets by sk_clone_lock().

The relevant ULP can take care of de-duplicating the context pointer via
the clone() operation, but only MPTCP and SMC implement such op.

Other ULPs may end-up with a double-free at socket disposal time.

We can't simply clear the ULP data at clone time, as TLS replaces the
socket ops with custom ones assuming a valid TLS ULP context is
available.

Instead completely prevent clone-less ULP sockets from entering the
LISTEN status.

Fixes: 734942c ("tcp: ULP infrastructure")
	Reported-by: slipper <[email protected]>
	Signed-off-by: Paolo Abeni <[email protected]>
Link: https://lore.kernel.org/r/4b80c3d1dbe3d0ab072f80450c202d9bc88b4b03.1672740602.git.pabeni@redhat.com
	Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit 2c02d41)
	Signed-off-by: Brett Mastbergen <[email protected]>
jira VULN-8789
cve-bf CVE-2023-0461
commit-author Paolo Abeni <[email protected]>
commit 8ccc993

The referenced commit changed the error code returned by the kernel
when preventing a non-established socket from attaching the ktls
ULP. Before to such a commit, the user-space got ENOTCONN instead
of EINVAL.

The existing self-tests depend on such error code, and the change
caused a failure:

  RUN           global.non_established ...
 tls.c:1673:non_established:Expected errno (22) == ENOTCONN (107)
 non_established: Test failed at step #3
          FAIL  global.non_established

In the unlikely event existing applications do the same, address
the issue by restoring the prior error code in the above scenario.

Note that the only other ULP performing similar checks at init
time - smc_ulp_ops - also fails with ENOTCONN when trying to attach
the ULP to a non-established socket.

	Reported-by: Sabrina Dubroca <[email protected]>
Fixes: 2c02d41 ("net/ulp: prevent ULP without clone op from entering the LISTEN status")
	Signed-off-by: Paolo Abeni <[email protected]>
	Reviewed-by: Sabrina Dubroca <[email protected]>
Link: https://lore.kernel.org/r/7bb199e7a93317fb6f8bf8b9b2dc71c18f337cde.1674042685.git.pabeni@redhat.com
	Signed-off-by: Jakub Kicinski <[email protected]>
(cherry picked from commit 8ccc993)
	Signed-off-by: Brett Mastbergen <[email protected]>

# Conflicts:
#	net/ipv4/tcp_ulp.c
Copy link

@thefossguy-ciq thefossguy-ciq left a comment

Choose a reason for hiding this comment

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

🚤

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:

@bmastbergen bmastbergen merged commit 2ebbad0 into fips-legacy-8-compliant/4.18.0-425.13.1 Jul 3, 2025
1 check passed
@bmastbergen bmastbergen deleted the bmastbergen_fips-legacy-8-compliant/4.18.0-425.13.1/VULN-8789 branch July 3, 2025 19:18
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