sunrpc: handle SVC_GARBAGE during svc auth processing as auth error

[shreeya-patel98]

• 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-71606
cve CVE-2025-38089
commit-author Jeff Layton <[email protected]>
commit 94d10a4dba0bc482f2b01e39f06d5513d0f75742
upstream-diff The following commits cause merge conflicts since they are
    not present in the ciqlts9_2 branch:
    6d037b15e439 ("SUNRPC: Remove the rpc_stat variable in svc_process_common()")
    ab42f4d9a26f ("sunrpc: don't change ->sv_stats if it doesn't exist")
    649a692e0f2b ("SUNRPC: Convert RPC Reply header encoding to use xdr_stream")

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: [email protected]
Fixes: 29cd2927fb91 ("SUNRPC: Fix encoding of accepted but unsuccessful RPC replies")
	Reported-by: tianshuo han <[email protected]>
	Reviewed-by: Chuck Lever <[email protected]>
	Signed-off-by: Jeff Layton <[email protected]>
	Signed-off-by: Chuck Lever <[email protected]>
(cherry picked from commit 94d10a4dba0bc482f2b01e39f06d5513d0f75742)
	Signed-off-by: Shreeya Patel <[email protected]>

Kernel build logs

/mnt/scratch/kernel-src-tree
Skipping mrproper (not requested)
[TIMER]{MRPROPER}: 0s
x86_64 architecture detected, copying config
'configs/kernel-x86_64-rhel.config' -> '.config'
Setting Local Version for build
CONFIG_LOCALVERSION="-_spatel__ciqlts9_2-9a0c8434ef40"
Making olddefconfig
#
# configuration written to .config
#
Starting Build
  SYNC    include/config/auto.conf.cmd
  UPD     include/config/kernel.release
  DESCEND objtool
  DESCEND bpf/resolve_btfids
  UPD     include/generated/utsrelease.h
  CALL    scripts/atomic/check-atomics.sh
warning: generated include/linux/atomic/atomic-instrumented.h has been modified.
  CALL    scripts/checksyscalls.sh
  CHK     include/generated/compile.h
  CC      init/version.o
  CC      arch/x86/crypto/aesni-intel_glue.o
  AR      init/built-in.a
  CC      kernel/sys.o
  CC      crypto/fips.o
  CC      security/integrity/ima/ima_init.o
  AR      arch/x86/crypto/built-in.a
  CC      crypto/algapi.o
  AR      arch/x86/built-in.a
  AR      security/integrity/ima/built-in.a
  AR      security/integrity/built-in.a
  AR      security/built-in.a
  CC      net/ethtool/ioctl.o
  CC      crypto/dh.o
  CC      crypto/rsa.o
  CC      crypto/rsa_helper.o
  CC      kernel/module.o
  CC      crypto/testmgr.o
  CC      drivers/base/firmware_loader/fallback_table.o
  CC      drivers/base/firmware_loader/main.o
  CC      drivers/base/firmware_loader/fallback.o
  CC [M]  fs/cifs/smbencrypt.o
  AR      net/ethtool/built-in.a
  CC      drivers/base/firmware_loader/sysfs.o
  CC      drivers/base/firmware_loader/sysfs_upload.o
  CC [M]  fs/cifs/cifsencrypt.o
  CC      crypto/hmac.o
  CC      drivers/base/firmware_loader/builtin/main.o
  AR      drivers/base/firmware_loader/builtin/built-in.a
  CC      crypto/xts.o
  CC [M]  drivers/nvme/target/admin-cmd.o
  AR      drivers/base/firmware_loader/built-in.a
  AR      drivers/base/built-in.a
  
  <--snip-->
  
    SIGN    /lib/modules/5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+/kernel/sound/xen/snd_xen_front.ko
  SIGN    /lib/modules/5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+/kernel/sound/virtio/virtio_snd.ko
  INSTALL /lib/modules/5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+/kernel/arch/x86/crypto/camellia-aesni-avx-x86_64.ko
  STRIP   /lib/modules/5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+/kernel/arch/x86/crypto/camellia-aesni-avx-x86_64.ko
  SIGN    /lib/modules/5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+/kernel/arch/x86/crypto/camellia-aesni-avx-x86_64.ko
  DEPMOD  /lib/modules/5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+
[TIMER]{MODULES}: 8s
Making Install
sh ./arch/x86/boot/install.sh \
	5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+ arch/x86/boot/bzImage \
	System.map "/boot"
[TIMER]{INSTALL}: 23s
Checking kABI
Checking kABI
kABI check passed
Setting Default Kernel to /boot/vmlinuz-5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+ and Index to 0
The default is /boot/loader/entries/2d84c760132f4bab9836f9dd9e3ac547-5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+.conf with index 0 and kernel /boot/vmlinuz-5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+
The default is /boot/loader/entries/2d84c760132f4bab9836f9dd9e3ac547-5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+.conf with index 0 and kernel /boot/vmlinuz-5.14.0-_spatel__ciqlts9_2-9a0c8434ef40+
Generating grub configuration file ...
Adding boot menu entry for UEFI Firmware Settings ...
done
Hopefully Grub2.0 took everything ... rebooting after time metrices
[TIMER]{MRPROPER}: 0s
[TIMER]{BUILD}: 214s
[TIMER]{MODULES}: 8s
[TIMER]{INSTALL}: 23s
[TIMER]{TOTAL} 250s
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
297
297
shreeya@spatel-dev-bom:~/ciq$ grep '^not ok ' kselftest-before.log | wc -l && grep '^not ok ' kselftest-after.log | wc -l
69
69

kselftest-before.log
kselftest-after.log

[thefossguy-ciq]

🚤

[PlaidCat]

Seems good, I'm not sure we'd be able to ingest the changes to err_bad_auth so its more consistent with 9.4.

It seem like all the use of rpc_stat that is pulled out of the err_garbage fall through is take care of else where but seems like its just in general a little dangerous.
It might be worth taking this as a pre-conditional (cve-pre) so that we don't have rely on a local variable.
6d037b1

I was also a little concerned about the difference in calls from err_bad_auth but to doeal with that we'd need at least these:
0ae060c
8dd41d7

These aren't blockers, but maybe something to consider

[shreeya-patel98]

@PlaidCat I had similar thoughts about making it consistent with 9.4, but I think for now it’s safer to integrate only this patch (the rpc_stat variable removal). This change is self-contained and has a lower risk of breaking anything.

I also tried applying the following two patches as part of a pre-cve.
ab42f4d ("sunrpc: don't change ->sv_stats if it doesn't exist")
649a692 ("SUNRPC: Convert RPC Reply header encoding to use xdr_stream")

But they don’t apply cleanly due to additional missing dependencies, and integrating them now might be risky since they introduce changes that we may not be able to test thoroughly.

I will update the PR to include a pre-cve patch for rpc_stat variable removal and test it. Thanks for the review.

[shreeya-patel98]

So I tried to add this patch 6d037b1 but kselftest is failing.

shreeya@spatel-dev-bom:~/ciq$ grep '^ok ' kselftest-before.log | wc -l && grep '^ok ' kselftest-after.log | wc -l
329
295
shreeya@spatel-dev-bom:~/ciq$ grep '^not ok ' kselftest-before.log | wc -l && grep '^not ok ' kselftest-after.log | wc -l
79
71

Attaching the patch that I added and kselftest logs. I will try to investigate it.

kselftest-before.log
kselftest-after.log

[PlaidCat]

I see, this is kinda what I thought might happen. None of the upstream shas for the ones you looked at are directly linked to a CVE so I think the investigation you did is sufficient.

Thanks for looking into it and seeing what things break or needed additional dependencies ... I don't want to go down the path of syncing this subsystem if it its a series of inter-dependent CVEs

Go ahead and merge when you're ready