[PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[Christian Borntraeger]

From: Christian Borntraeger <borntraeger@linux.ibm.com>

With variable length and EXTENDED_LENGTH_SCCB, some callbacks might
change the length field. For example read SCP info might write a new
length into the SCCB header. We must not use that new length for the
buffer copy, since the buffer was allocated with the original length.
Only the length field in the work SCCB is changed, to indicate the
"necessary" size. Using the new length reads past the allocation, so
tools like ASAN might detect a buffer overrun.

Secure guests do not have EXTENDED_LENGTH_SCCB, and the ultravisor checks
and sanitizes the length field, so no qemu heap contents are exposed to
the guest and the non pv-path already has the same header.length.

Fixes: 0f73c5b30b8b ("s390x: protvirt: SCLP interpretation")
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
 hw/s390x/sclp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 3c8cb164888..b452f2ce54d 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -287,7 +287,7 @@ int sclp_service_call_protected(S390CPU *cpu, uint64_t sccb, uint32_t code)
     sclp_c->execute(sclp, work_sccb, code);
 out_write:
     s390_cpu_pv_mem_write(env_archcpu(env), 0, work_sccb,
-                          be16_to_cpu(work_sccb->h.length));
+                          be16_to_cpu(header.length));
     sclp_c->service_interrupt(sclp, SCLP_PV_DUMMY_ADDR);
     return 0;
 }
-- 
2.55.0

Re: [PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[Eric Farman]

On 8/14/26 8:58 AM, Christian Borntraeger wrote:
> From: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> With variable length and EXTENDED_LENGTH_SCCB, some callbacks might
> change the length field. For example read SCP info might write a new
> length into the SCCB header. We must not use that new length for the
> buffer copy, since the buffer was allocated with the original length.
> Only the length field in the work SCCB is changed, to indicate the
> "necessary" size. Using the new length reads past the allocation, so
> tools like ASAN might detect a buffer overrun.
> 
> Secure guests do not have EXTENDED_LENGTH_SCCB, and the ultravisor checks
> and sanitizes the length field, so no qemu heap contents are exposed to
> the guest and the non pv-path already has the same header.length.
> 
> Fixes: 0f73c5b30b8b ("s390x: protvirt: SCLP interpretation")
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>   hw/s390x/sclp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Farman <farman@linux.ibm.com>

Re: [PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[Matthew Rosato]

On 8/14/26 8:58 AM, Christian Borntraeger wrote:
> From: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> With variable length and EXTENDED_LENGTH_SCCB, some callbacks might
> change the length field. For example read SCP info might write a new
> length into the SCCB header. We must not use that new length for the
> buffer copy, since the buffer was allocated with the original length.
> Only the length field in the work SCCB is changed, to indicate the
> "necessary" size. Using the new length reads past the allocation, so
> tools like ASAN might detect a buffer overrun.
> 
> Secure guests do not have EXTENDED_LENGTH_SCCB, and the ultravisor checks
> and sanitizes the length field, so no qemu heap contents are exposed to
> the guest and the non pv-path already has the same header.length.
> 
> Fixes: 0f73c5b30b8b ("s390x: protvirt: SCLP interpretation")
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

Re: [PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[Eric Farman]

On 8/14/26 8:58 AM, Christian Borntraeger wrote:
> From: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> With variable length and EXTENDED_LENGTH_SCCB, some callbacks might
> change the length field. For example read SCP info might write a new
> length into the SCCB header. We must not use that new length for the
> buffer copy, since the buffer was allocated with the original length.
> Only the length field in the work SCCB is changed, to indicate the
> "necessary" size. Using the new length reads past the allocation, so
> tools like ASAN might detect a buffer overrun.
> 
> Secure guests do not have EXTENDED_LENGTH_SCCB, and the ultravisor checks
> and sanitizes the length field, so no qemu heap contents are exposed to
> the guest and the non pv-path already has the same header.length.
> 
> Fixes: 0f73c5b30b8b ("s390x: protvirt: SCLP interpretation")
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>   hw/s390x/sclp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Thanks! Applied for 11.2

Re: [PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[Philippe Mathieu-Daudé]

On 19/8/26 13:06, Eric Farman wrote:
> 
> 
> On 8/14/26 8:58 AM, Christian Borntraeger wrote:
>> From: Christian Borntraeger <borntraeger@linux.ibm.com>
>>
>> With variable length and EXTENDED_LENGTH_SCCB, some callbacks might
>> change the length field. For example read SCP info might write a new
>> length into the SCCB header. We must not use that new length for the
>> buffer copy, since the buffer was allocated with the original length.
>> Only the length field in the work SCCB is changed, to indicate the
>> "necessary" size. Using the new length reads past the allocation, so
>> tools like ASAN might detect a buffer overrun.
>>
>> Secure guests do not have EXTENDED_LENGTH_SCCB, and the ultravisor checks
>> and sanitizes the length field, so no qemu heap contents are exposed to
>> the guest and the non pv-path already has the same header.length.
>>

Cc: qemu-stable@nongnu.org

>> Fixes: 0f73c5b30b8b ("s390x: protvirt: SCLP interpretation")
>> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>> ---
>>   hw/s390x/sclp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks! Applied for 11.2
> 

Re: [PATCH] s390x/sclp: pv: only copy the original SCCB buffer

[Michael Tokarev]

On 8/14/26 15:58, Christian Borntraeger wrote:
> From: Christian Borntraeger <borntraeger@linux.ibm.com>
> 
> With variable length and EXTENDED_LENGTH_SCCB, some callbacks might
> change the length field. For example read SCP info might write a new
> length into the SCCB header. We must not use that new length for the
> buffer copy, since the buffer was allocated with the original length.
> Only the length field in the work SCCB is changed, to indicate the
> "necessary" size. Using the new length reads past the allocation, so
> tools like ASAN might detect a buffer overrun.
> 
> Secure guests do not have EXTENDED_LENGTH_SCCB, and the ultravisor checks
> and sanitizes the length field, so no qemu heap contents are exposed to
> the guest and the non pv-path already has the same header.length.
> 
> Fixes: 0f73c5b30b8b ("s390x: protvirt: SCLP interpretation")
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>   hw/s390x/sclp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

I'm picking this one up for the currently active stable qemu series.
Please let me know if I shouldn't.

Thanks,

/mjt