On 7/9/26 10:29, Christian Borntraeger wrote:
> We verify the sccb length and then allocate based on that length. The
> following access re-reads the sccb again. This can race against other
> vCPUs overwriting the length field.
>
> sclp_service_call_protected does not need a change as the ultravisor
> provides a consistent snapshot.
>
> Fixes: c1db53a5910f ("s390/sclp: read sccb from mem based on provided length")
> Cc: [email protected]
> Signed-off-by: Christian Borntraeger <[email protected]>
> Reviewed-by: Matthew Rosato <[email protected]>
> ---
> hw/s390x/sclp.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 2d2cde7803..3c8cb16488 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -329,7 +329,8 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb,
> uint32_t code)
> /*
> * we want to work on a private copy of the sccb, to prevent guests
> * from playing dirty tricks by modifying the memory content after
> - * the host has checked the values
> + * the host has checked the values.
> + * Reuse the previously fetched header
> */
> work_sccb = g_malloc0(be16_to_cpu(header.length));
> ret = address_space_read(as, sccb, attrs,
> @@ -337,6 +338,7 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb,
> uint32_t code)
> if (ret != MEMTX_OK) {
> return -PGM_ADDRESSING;
> }
> + work_sccb->h = header;
>
> if (!sclp_command_code_valid(code)) {
> work_sccb->h.response_code =
> cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
I guess the alternative (and messier) approach would be to re-validate
the new header. This proposal looks good to me.
Reviewed-by: Collin Walling <[email protected]>
--
Regards,
Collin
