Re: [PATCH v2 1/1] s390/pkey: Rework ioctl functions error pathes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Passing ERR_PTR values from `_copy_key_from_user` or `_copy_apqns_from_user` to `kfree()` and `kfree_sensitive()` causes a deterministic kernel panic.
- [Critical] Accidental removal of the `ktp.pkeylen` update in `pkey_ioctl_kblob2protk3` breaks the userspace ABI.
--

Note: Due to recitation restrictions, this report does not use the standard
inline quoting format to show the modified code. Instead, I am providing a
free-form summary of the issues found in the patch.

commit 280086862168752a0b3725329891a790006a089e
Author: Harald Freudenberger <freude@linux.ibm.com>
s390/pkey: Rework ioctl functions error pathes

This commit refactors the error handling and free paths in the pkey ioctl
functions. It replaces duplicated cleanup code with centralized goto labels
to handle freeing memory and zeroing operations at the end of each function.


[Severity: Critical]
When _copy_key_from_user() or _copy_apqns_from_user() fails and returns an
ERR_PTR, does the jump to the out label cause kfree() or kfree_sensitive() 
to dereference that ERR_PTR?

In drivers/s390/crypto/pkey_api.c, several ioctl functions check for IS_ERR()
after copying from user space. If an error occurs, they assign rc and then 
execute goto out. However, at the out label, the pointer is unconditionally 
passed to the freeing functions. Passing an ERR_PTR (such as -EFAULT) to the
kfree family causes the slab allocator to attempt dereferencing an invalid 
address, resulting in a kernel panic. 

This pattern appears to be introduced across multiple functions, including
pkey_ioctl_kblob2protk(), pkey_ioctl_clr2seck2(), pkey_ioctl_verifykey2(),
pkey_ioctl_kblob2protk2(), pkey_ioctl_apqns4k(), and
pkey_ioctl_kblob2protk3().


[Severity: Critical]
Was the assignment of ktp.pkeylen intentionally removed in 
pkey_ioctl_kblob2protk3()? 

During the refactoring in drivers/s390/crypto/pkey_api.c, the line 
ktp.pkeylen = protkeylen; was dropped right before the final copy_to_user() 
call. Dropping this assignment prevents the kernel from returning the actual 
size of the derived key back to userspace. Will this break the ABI for 
applications relying on this size query?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703094618.5916-2-freude@linux.ibm.com?part=1