Re: [GIT PULL v1 11/11] s390/uv: Prevent potential out-of-bounds read

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential infinite loop in `uvio_get_list` if UVC hardware returns `UVC_RC_MORE_DATA` but fails to advance `next_secret_idx`.
- [High] Missing bounds check on `num_secr_stored` against array size 85, leading to potential out-of-bounds reads and information leaks.
--

commit 3bddf93bb1450f001c28919a0ec81368564fe49c
Author: Steffen Eiden <seiden@linux.ibm.com>

s390/uv: Prevent potential out-of-bounds read

When the system has more than 85 secrets, the uv_secret_list struct array
only holds up to 85 items per page, resulting in an out of bounds read in
find_secret_in_page if the targeted secret is in the next page or not
stored at all. Fix this by looping over the per sub-list count of stored
secrets instead of the overall count.

> diff --git a/arch/s390/kernel/uv.c b/arch/s390/kernel/uv.c
> index 52003c463fad0..8ea9dd7704ffc 100644
> --- a/arch/s390/kernel/uv.c
> +++ b/arch/s390/kernel/uv.c
> @@ -825,7 +825,7 @@ static int find_secret_in_page(const u8 secret_id[UV_SECRET_ID_LEN],
>  {
>  	u16 i;
>  
> -	for (i = 0; i < list->total_num_secrets; i++) {
> +	for (i = 0; i < list->num_secr_stored; i++) {

[Severity: High]
This is a pre-existing issue, but does list->num_secr_stored need to be
bounds checked against the size of the secrets[] array (85) before being
used as a loop limit?

If the Ultravisor firmware returns a num_secr_stored value greater than 85,
could this still result in an out-of-bounds read in find_secret_in_page()?

This same missing bounds check appears in uvio_get_list():

drivers/s390/char/uvdevice.c:uvio_get_list() {
    ...
		copy_len = sizeof(list->secrets[0]) * list->num_secr_stored;
		if (copy_to_user(user_buf + user_off, list->secrets, copy_len))
			return -EFAULT;
    ...
}

Could an anomalously large num_secr_stored here cause copy_to_user() to copy
kernel memory beyond the allocated page out to user-space?

[Severity: High]
This is also a pre-existing issue, but looking at uvio_get_list() in
drivers/s390/char/uvdevice.c, is there a potential infinite loop?

drivers/s390/char/uvdevice.c:uvio_get_list() {
    ...
	do {
		uv_list_secrets(list, list->next_secret_idx, &uv_ioctl->uv_rc,
				&uv_ioctl->uv_rrc);
		if (uv_ioctl->uv_rc != UVC_RC_EXECUTED &&
		    uv_ioctl->uv_rc != UVC_RC_MORE_DATA)
			break;

		copy_len = sizeof(list->secrets[0]) * list->num_secr_stored;
		...
		user_off += copy_len;
		num_secrets_stored += list->num_secr_stored;
	} while (uv_ioctl->uv_rc == UVC_RC_MORE_DATA &&
		 user_off + sizeof(*list) <= uv_ioctl->argument_len);
    ...
}

If the hardware/firmware returns UVC_RC_MORE_DATA but yields
list->num_secr_stored == 0, user_off does not increment. If
list->next_secret_idx is also not advanced by the hardware, could this loop
indefinitely and cause a CPU stall?

>  		if (memcmp(secret_id, list->secrets[i].id, UV_SECRET_ID_LEN) == 0) {
>  			*secret = list->secrets[i].hdr;
>  			return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904060647.562443-1-imbrenda@linux.ibm.com?part=11