* [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors
@ 2025-10-07 1:58 Philippe Mathieu-Daudé
2025-10-08 12:20 ` Thomas Huth
2025-10-08 14:41 ` Jason J. Herne
0 siblings, 2 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-07 1:58 UTC (permalink / raw)
To: qemu-devel
Cc: Ilya Leoshkevich, David Hildenbrand, Thomas Huth, Jason Herne,
Halil Pasic, qemu-s390x, Eric Farman, Richard Henderson,
Christian Borntraeger, Matthew Rosato,
Philippe Mathieu-Daudé
If address_space_read() fails, return PGM_ADDRESSING. In the
unlikely case address_space_write() fails (we already checked
the address is readable), return PGM_PROTECTION.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Based-on: <20251007014958.19086-1-philmd@linaro.org>
---
hw/s390x/sclp.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index d7cb99482b2..8604cd305e5 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -305,6 +305,7 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
SCCBHeader header;
g_autofree SCCB *work_sccb = NULL;
AddressSpace *as = CPU(cpu)->as;
+ MemTxResult ret;
/* first some basic checks on program checks */
if (env->psw.mask & PSW_MASK_PSTATE) {
@@ -319,8 +320,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
}
/* the header contains the actual length of the sccb */
- address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
- &header, sizeof(SCCBHeader));
+ ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
+ &header, sizeof(SCCBHeader));
+ if (ret != MEMTX_OK) {
+ return -PGM_ADDRESSING;
+ }
/* Valid sccb sizes */
if (be16_to_cpu(header.length) < sizeof(SCCBHeader)) {
@@ -333,8 +337,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
* the host has checked the values
*/
work_sccb = g_malloc0(be16_to_cpu(header.length));
- address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
- work_sccb, be16_to_cpu(header.length));
+ ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
+ work_sccb, be16_to_cpu(header.length));
+ if (ret != MEMTX_OK) {
+ return -PGM_ADDRESSING;
+ }
if (!sclp_command_code_valid(code)) {
work_sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
@@ -348,8 +355,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
sclp_c->execute(sclp, work_sccb, code);
out_write:
- address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
- work_sccb, be16_to_cpu(header.length));
+ ret = address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
+ work_sccb, be16_to_cpu(header.length));
+ if (ret != MEMTX_OK) {
+ return -PGM_PROTECTION;
+ }
sclp_c->service_interrupt(sclp, sccb);
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors
2025-10-07 1:58 [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors Philippe Mathieu-Daudé
@ 2025-10-08 12:20 ` Thomas Huth
2025-10-08 14:41 ` Jason J. Herne
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Huth @ 2025-10-08 12:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Ilya Leoshkevich, David Hildenbrand, Jason Herne, Halil Pasic,
qemu-s390x, Eric Farman, Richard Henderson, Christian Borntraeger,
Matthew Rosato
On 07/10/2025 03.58, Philippe Mathieu-Daudé wrote:
> If address_space_read() fails, return PGM_ADDRESSING. In the
> unlikely case address_space_write() fails (we already checked
> the address is readable), return PGM_PROTECTION.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20251007014958.19086-1-philmd@linaro.org>
> ---
> hw/s390x/sclp.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index d7cb99482b2..8604cd305e5 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -305,6 +305,7 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
> SCCBHeader header;
> g_autofree SCCB *work_sccb = NULL;
> AddressSpace *as = CPU(cpu)->as;
> + MemTxResult ret;
>
> /* first some basic checks on program checks */
> if (env->psw.mask & PSW_MASK_PSTATE) {
> @@ -319,8 +320,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
> }
>
> /* the header contains the actual length of the sccb */
> - address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> - &header, sizeof(SCCBHeader));
> + ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> + &header, sizeof(SCCBHeader));
> + if (ret != MEMTX_OK) {
> + return -PGM_ADDRESSING;
> + }
>
> /* Valid sccb sizes */
> if (be16_to_cpu(header.length) < sizeof(SCCBHeader)) {
> @@ -333,8 +337,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
> * the host has checked the values
> */
> work_sccb = g_malloc0(be16_to_cpu(header.length));
> - address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> - work_sccb, be16_to_cpu(header.length));
> + ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> + work_sccb, be16_to_cpu(header.length));
> + if (ret != MEMTX_OK) {
> + return -PGM_ADDRESSING;
> + }
>
> if (!sclp_command_code_valid(code)) {
> work_sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> @@ -348,8 +355,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
>
> sclp_c->execute(sclp, work_sccb, code);
> out_write:
> - address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
> - work_sccb, be16_to_cpu(header.length));
> + ret = address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
> + work_sccb, be16_to_cpu(header.length));
> + if (ret != MEMTX_OK) {
> + return -PGM_PROTECTION;
> + }
>
> sclp_c->service_interrupt(sclp, sccb);
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors
2025-10-07 1:58 [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors Philippe Mathieu-Daudé
2025-10-08 12:20 ` Thomas Huth
@ 2025-10-08 14:41 ` Jason J. Herne
1 sibling, 0 replies; 3+ messages in thread
From: Jason J. Herne @ 2025-10-08 14:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Ilya Leoshkevich, David Hildenbrand, Thomas Huth, Halil Pasic,
qemu-s390x, Eric Farman, Richard Henderson, Christian Borntraeger,
Matthew Rosato
On 10/6/25 9:58 PM, Philippe Mathieu-Daudé wrote:
> If address_space_read() fails, return PGM_ADDRESSING. In the
> unlikely case address_space_write() fails (we already checked
> the address is readable), return PGM_PROTECTION.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20251007014958.19086-1-philmd@linaro.org>
> ---
> hw/s390x/sclp.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index d7cb99482b2..8604cd305e5 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -305,6 +305,7 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
> SCCBHeader header;
> g_autofree SCCB *work_sccb = NULL;
> AddressSpace *as = CPU(cpu)->as;
> + MemTxResult ret;
>
> /* first some basic checks on program checks */
> if (env->psw.mask & PSW_MASK_PSTATE) {
> @@ -319,8 +320,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
> }
>
> /* the header contains the actual length of the sccb */
> - address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> - &header, sizeof(SCCBHeader));
> + ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> + &header, sizeof(SCCBHeader));
> + if (ret != MEMTX_OK) {
> + return -PGM_ADDRESSING;
> + }
>
> /* Valid sccb sizes */
> if (be16_to_cpu(header.length) < sizeof(SCCBHeader)) {
> @@ -333,8 +337,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
> * the host has checked the values
> */
> work_sccb = g_malloc0(be16_to_cpu(header.length));
> - address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> - work_sccb, be16_to_cpu(header.length));
> + ret = address_space_read(as, sccb, MEMTXATTRS_UNSPECIFIED,
> + work_sccb, be16_to_cpu(header.length));
> + if (ret != MEMTX_OK) {
> + return -PGM_ADDRESSING;
> + }
>
> if (!sclp_command_code_valid(code)) {
> work_sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> @@ -348,8 +355,11 @@ int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code)
>
> sclp_c->execute(sclp, work_sccb, code);
> out_write:
> - address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
> - work_sccb, be16_to_cpu(header.length));
> + ret = address_space_write(as, sccb, MEMTXATTRS_UNSPECIFIED,
> + work_sccb, be16_to_cpu(header.length));
> + if (ret != MEMTX_OK) {
> + return -PGM_PROTECTION;
> + }
>
> sclp_c->service_interrupt(sclp, sccb);
>
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-08 14:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-07 1:58 [RFC PATCH] hw/s390x/sclp: Do not ignore address_space_read/write() errors Philippe Mathieu-Daudé
2025-10-08 12:20 ` Thomas Huth
2025-10-08 14:41 ` Jason J. Herne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).