Skip to content

[ciqlts9_4] sunrpc: handle SVC_GARBAGE during svc auth processing as auth error #396

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

Merged

Conversation

shreeya-patel98
Copy link

  • Commit Message Requirements
  • Built against Vault/LTS Environment
  • kABI Check Passed, where Valid (Pre 9.4 RT does not have kABI stability)
  • Boot Test
  • Kernel SelfTest results
  • Additional Tests as determined relevant

Commit message

jira VULN-71607
cve CVE-2025-38089
commit-author Jeff Layton <jlayton@kernel.org>
commit 94d10a4dba0bc482f2b01e39f06d5513d0f75742
upstream-diff A merge conflict was caused because the following
    commit doesn't exit in our tree:
    ab42f4d9a26f ("sunrpc: don't change ->sv_stats if it doesn't exist")

tianshuo han reported a remotely-triggerable crash if the client sends a kernel RPC server a specially crafted packet. If decoding the RPC reply fails in such a way that SVC_GARBAGE is returned without setting the rq_accept_statp pointer, then that pointer can be dereferenced and a value stored there.

If it's the first time the thread has processed an RPC, then that pointer will be set to NULL and the kernel will crash. In other cases, it could create a memory scribble.

The server sunrpc code treats a SVC_GARBAGE return from svc_authenticate or pg_authenticate as if it should send a GARBAGE_ARGS reply. RFC 5531 says that if authentication fails that the RPC should be rejected instead with a status of AUTH_ERR.

Handle a SVC_GARBAGE return as an AUTH_ERROR, with a reason of AUTH_BADCRED instead of returning GARBAGE_ARGS in that case. This sidesteps the whole problem of touching the rpc_accept_statp pointer in this situation and avoids the crash.

	Cc: stable@kernel.org
Fixes: 29cd2927fb91 ("SUNRPC: Fix encoding of accepted but unsuccessful RPC replies")
	Reported-by: tianshuo han <hantianshuo233@gmail.com>
	Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
	Signed-off-by: Jeff Layton <jlayton@kernel.org>
	Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
(cherry picked from commit 94d10a4dba0bc482f2b01e39f06d5513d0f75742)
	Signed-off-by: Shreeya Patel <spatel@ciq.com>

Kernel build logs

/mnt/scratch/kernel-src-tree
  CLEAN   arch/x86/boot/compressed
  CLEAN   arch/x86/boot
  CLEAN   arch/x86/crypto
  CLEAN   arch/x86/entry/vdso
  CLEAN   arch/x86/kernel/cpu
  CLEAN   arch/x86/kernel
  CLEAN   arch/x86/kvm
  CLEAN   arch/x86/purgatory
  CLEAN   arch/x86/realmode/rm
  CLEAN   arch/x86/tools
  CLEAN   arch/x86/lib
  CLEAN   certs
  CLEAN   crypto/asymmetric_keys
  CLEAN   drivers/firmware/efi/libstub
  CLEAN   drivers/gpu/drm/radeon
  CLEAN   drivers/scsi
  CLEAN   drivers/tty/vt
  CLEAN   drivers/video/logo
  CLEAN   kernel/debug/kdb
  CLEAN   kernel
  CLEAN   lib/raid6
  CLEAN   lib
  CLEAN   net/wireless
  CLEAN   security/selinux
  CLEAN   usr/include
  CLEAN   usr
  CLEAN   vmlinux.symvers modules-only.symvers modules.builtin modules.builtin.modinfo
  CLEAN   scripts/basic
  CLEAN   scripts/genksyms
  CLEAN   scripts/kconfig
  CLEAN   scripts/mod
  CLEAN   scripts/selinux/genheaders
  CLEAN   scripts/selinux/mdp
  CLEAN   scripts
  CLEAN   include/config include/generated arch/x86/include/generated .config .config.old .version Module.symvers certs/signing_key.pem certs/signing_key.x509 certs/x509.genkey
[TIMER]{MRPROPER}: 9s
x86_64 architecture detected, copying config
'configs/kernel-x86_64-rhel.config' -> '.config'
Setting Local Version for build
CONFIG_LOCALVERSION="-spatel_ciqlts9_4-9e63bed61646"
Making olddefconfig
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  LEX     scripts/kconfig/lexer.lex.c
  YACC    scripts/kconfig/parser.tab.[ch]
  HOSTCC  scripts/kconfig/lexer.lex.o
  HOSTCC  scripts/kconfig/menu.o
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTCC  scripts/kconfig/util.o
  HOSTLD  scripts/kconfig/conf
#
# configuration written to .config
#
Starting Build
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_32.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_64.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_x32.h
  
  <--snip-->
  
    STRIP   /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/usb/snd-usb-audio.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/usb/usx2y/snd-usb-usx2y.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/usb/usx2y/snd-usb-us122l.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/virtio/virtio_snd.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/x86/snd-hdmi-lpe-audio.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/xen/snd_xen_front.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/virt/lib/irqbypass.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+/kernel/sound/usb/snd-usb-audio.ko
  DEPMOD  /lib/modules/5.14.0-spatel_ciqlts9_4-9e63bed61646+
[TIMER]{MODULES}: 9s
Making Install
sh ./arch/x86/boot/install.sh 5.14.0-spatel_ciqlts9_4-9e63bed61646+ \
	arch/x86/boot/bzImage System.map "/boot"
[TIMER]{INSTALL}: 22s
Checking kABI
Checking kABI
kABI check passed
Setting Default Kernel to /boot/vmlinuz-5.14.0-spatel_ciqlts9_4-9e63bed61646+ and Index to 2
The default is /boot/loader/entries/216206ccc95d48aa9c1f435aaa394c07-5.14.0-spatel_ciqlts9_4-9e63bed61646+.conf with index 2 and kernel /boot/vmlinuz-5.14.0-spatel_ciqlts9_4-9e63bed61646+
The default is /boot/loader/entries/216206ccc95d48aa9c1f435aaa394c07-5.14.0-spatel_ciqlts9_4-9e63bed61646+.conf with index 2 and kernel /boot/vmlinuz-5.14.0-spatel_ciqlts9_4-9e63bed61646+
Generating grub configuration file ...
Adding boot menu entry for UEFI Firmware Settings ...
done
Hopefully Grub2.0 took everything ... rebooting after time metrices
[TIMER]{MRPROPER}: 9s
[TIMER]{BUILD}: 1414s
[TIMER]{MODULES}: 9s
[TIMER]{INSTALL}: 22s
[TIMER]{TOTAL} 1462s
Rebooting in 10 seconds

kernel-build.log

Kselftests

shreeya@spatel-dev-bom:~/ciq$  grep '^ok ' kselftest-before.log | wc -l && grep '^ok ' kselftest-after.log | wc -l
340
341
shreeya@spatel-dev-bom:~/ciq$ grep '^not ok ' kselftest-before.log | wc -l && grep '^not ok ' kselftest-after.log | wc -l
75
74

kselftest-before.log
kselftest-after.log

@shreeya-patel98 shreeya-patel98 requested a review from Copilot July 7, 2025 13:11
@shreeya-patel98 shreeya-patel98 self-assigned this Jul 7, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the handling of SVC_GARBAGE in svc_process_common so that it is treated as an authentication error (AUTH_BADCRED) rather than returning GARBAGE_ARGS, in order to avoid dereferencing a null rq_accept_statp and to comply with RFC 5531.

  • Replace err_garbage_args path with setting rq_auth_stat = rpc_autherr_badcred and jumping to err_bad_auth.
  • Remove the old err_garbage_args label block and its stats/log updates.
  • Ensure crashes from malformed packets no longer occur by sidestepping rq_accept_statp.
Comments suppressed due to low confidence (2)

net/sunrpc/svc.c:1345

  • [nitpick] Consider updating or adding a comment above this case to note that SVC_GARBAGE now maps to an auth error (AUTH_BADCRED) per RFC 5531, replacing the old garbage-args behavior.
	case SVC_GARBAGE:

net/sunrpc/svc.c:1346

  • Add a kselftest or integration test that triggers the SVC_GARBAGE path to verify the request is rejected with AUTH_BADCRED instead of GARBAGE_ARGS.
		rqstp->rq_auth_stat = rpc_autherr_badcred;

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.

A minor nit pick: Your commit message has two Signed-off-by from you. I assume you added the Signed-off-by at the very bottom by habit. But, that isn't necessary becasue the ciq-cherry-pick.py script already does it.

jira VULN-71607
cve CVE-2025-38089
commit-author Jeff Layton <jlayton@kernel.org>
commit 94d10a4
upstream-diff A merge conflict was caused because the following
    commit doesn't exit in our tree:
    ab42f4d ("sunrpc: don't change ->sv_stats if it doesn't exist")

tianshuo han reported a remotely-triggerable crash if the client sends a
kernel RPC server a specially crafted packet. If decoding the RPC reply
fails in such a way that SVC_GARBAGE is returned without setting the
rq_accept_statp pointer, then that pointer can be dereferenced and a
value stored there.

If it's the first time the thread has processed an RPC, then that
pointer will be set to NULL and the kernel will crash. In other cases,
it could create a memory scribble.

The server sunrpc code treats a SVC_GARBAGE return from svc_authenticate
or pg_authenticate as if it should send a GARBAGE_ARGS reply. RFC 5531
says that if authentication fails that the RPC should be rejected
instead with a status of AUTH_ERR.

Handle a SVC_GARBAGE return as an AUTH_ERROR, with a reason of
AUTH_BADCRED instead of returning GARBAGE_ARGS in that case. This
sidesteps the whole problem of touching the rpc_accept_statp pointer in
this situation and avoids the crash.

	Cc: stable@kernel.org
Fixes: 29cd292 ("SUNRPC: Fix encoding of accepted but unsuccessful RPC replies")
	Reported-by: tianshuo han <hantianshuo233@gmail.com>
	Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
	Signed-off-by: Jeff Layton <jlayton@kernel.org>
	Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
(cherry picked from commit 94d10a4)
	Signed-off-by: Shreeya Patel <spatel@ciq.com>
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:

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.

Great first PR! We have one more kselftest passing which is nice. 🚤

@shreeya-patel98 shreeya-patel98 merged commit 389d406 into ctrliq:ciqlts9_4 Jul 8, 2025
4 checks passed
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