Re: [PATCH v5 2/4] s390/sclp_tty: Make use of sclp_gds_for_each()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/sclp: out-of-bounds read in sclp_gds_for_each() due to integer promotion
--

commit b02b1435e7ea0c6d35613a83d6a0bafa8d59fb95
Author: Alexander Egorenkov <egorenar@linux.ibm.com>

s390/sclp_tty: Make use of sclp_gds_for_each()

This commit replaces manual iterations over gds subvector entries with the
sclp_gds_for_each() macro. The stated goal is to handle erroneous entries
in gds vectors and subvectors more safely.

> diff --git a/drivers/s390/char/sclp_tty.c b/drivers/s390/char/sclp_tty.c
> index 0a92d08830e7c..4353890bbf8af 100644
> --- a/drivers/s390/char/sclp_tty.c
> +++ b/drivers/s390/char/sclp_tty.c
> @@ -418,38 +418,38 @@ static void sclp_get_input(struct gds_subvector *sv)
>  
>  static inline void sclp_eval_selfdeftextmsg(struct gds_subvector *sv)
>  {
> -	void *end;
> -
> -	end = (void *) sv + sv->length;
> -	for (sv = sv + 1; (void *) sv < end; sv = (void *) sv + sv->length)
> +	void *start = sv + 1;
> +	void *end = (void *) sv + sv->length;
> +	int n;
> +	sclp_gds_for_each(sv, n, start, end) {

[Severity: High]
Does this code introduce an out-of-bounds read regression if the hardware
provides a malformed sccb buffer with a length smaller than the header size?

If sv->length is smaller than sizeof(*sv), start will be greater than end.
In the sclp_gds_for_each() macro defined in drivers/s390/char/sclp.h, n is
defined as an int and evaluated as n = (end) - (start), which becomes a
negative value:

drivers/s390/char/sclp.h:
#define sclp_gds_for_each(v, n, start, end) \
	for ((n) = (end) - (start), (v) = (start); \
	     (n) >= sizeof(*(v)) && (v)->length >= sizeof(*(v)) && (v)->length <= (n); \
	     (n) -= (v)->length, (v) = (void*)(v) + (v)->length)

Because sizeof returns an unsigned size_t, standard C integer promotion
rules convert the negative int n to a very large positive size_t value
when evaluating (n) >= sizeof(*(v)). This incorrectly bypasses the initial
bounds check.

The condition then proceeds to evaluate (v)->length >= sizeof(*(v)),
dereferencing v. Since v points to start which is beyond end, this results
in an out-of-bounds memory read (1 or 2 bytes), which could cause a kernel
panic due to a page fault on unmapped memory or trigger kasan.

This regression is exposed here in sclp_eval_selfdeftextmsg(), but also
directly in sclp_tty_receiver() when parsing untrusted sclp event buffers
from the hardware or hypervisor, where an evbuf->length smaller than
sizeof(*evbuf) causes start > end:

drivers/s390/char/sclp_tty.c:sclp_tty_receiver() {
    ...
    v = sclp_find_gds_vector(evbuf + 1, (void *) evbuf + evbuf->length,
                             GDS_ID_MDSMU);
    ...
}

>  		if (sv->key == 0x30)
>  			sclp_get_input(sv);
> +	}
>  }

[ ... ]

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