[v6] Secure IPL Support for SCSI Scheme of virtio-blk/virtio-scsi Devices

[PATCH v6 06/28] s390x/diag: Refactor address validation check from diag308_parm_check

Posted by Zhuoying Cai 1 month, 4 weeks ago

Create a function to validate the address parameter of DIAGNOSE.

Refactor the function for reuse in the next patch, which allows address
validation in read or write operation of DIAGNOSE.

Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
---
 hw/s390x/ipl.h      | 6 ++++++
 target/s390x/diag.c | 4 +---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index bee72dfbb3..e26fc1cd6a 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -118,6 +118,12 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
 #define S390_IPLB_MIN_FCP_LEN 384
 #define S390_IPLB_MIN_QEMU_SCSI_LEN 200
 
+static inline bool diag_parm_addr_valid(uint64_t addr, size_t size, bool write)
+{
+    return address_space_access_valid(&address_space_memory, addr,
+                                      size, write, MEMTXATTRS_UNSPECIFIED);
+}
+
 static inline bool iplb_valid_len(IplParameterBlock *iplb)
 {
     return be32_to_cpu(iplb->len) <= sizeof(IplParameterBlock);
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index a35d808fd7..e67ee57f01 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -65,9 +65,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return -1;
     }
-    if (!address_space_access_valid(&address_space_memory, addr,
-                                    sizeof(IplParameterBlock), write,
-                                    MEMTXATTRS_UNSPECIFIED)) {
+    if (!diag_parm_addr_valid(addr, sizeof(IplParameterBlock), write)) {
         s390_program_interrupt(env, PGM_ADDRESSING, ra);
         return -1;
     }
-- 
2.50.1

Re: [PATCH v6 06/28] s390x/diag: Refactor address validation check from diag308_parm_check

Posted by Thomas Huth 1 month, 2 weeks ago

On 18/09/2025 01.21, Zhuoying Cai wrote:
> Create a function to validate the address parameter of DIAGNOSE.
> 
> Refactor the function for reuse in the next patch, which allows address
> validation in read or write operation of DIAGNOSE.
> 
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   hw/s390x/ipl.h      | 6 ++++++
>   target/s390x/diag.c | 4 +---
>   2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index bee72dfbb3..e26fc1cd6a 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -118,6 +118,12 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
>   #define S390_IPLB_MIN_FCP_LEN 384
>   #define S390_IPLB_MIN_QEMU_SCSI_LEN 200
>   
> +static inline bool diag_parm_addr_valid(uint64_t addr, size_t size, bool write)
> +{
> +    return address_space_access_valid(&address_space_memory, addr,
> +                                      size, write, MEMTXATTRS_UNSPECIFIED);
> +}

The function is only used in diag.c ... could you please move it to that file?

  Thanks,
   Thomas


>   static inline bool iplb_valid_len(IplParameterBlock *iplb)
>   {
>       return be32_to_cpu(iplb->len) <= sizeof(IplParameterBlock);
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index a35d808fd7..e67ee57f01 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -65,9 +65,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
>           s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>           return -1;
>       }
> -    if (!address_space_access_valid(&address_space_memory, addr,
> -                                    sizeof(IplParameterBlock), write,
> -                                    MEMTXATTRS_UNSPECIFIED)) {
> +    if (!diag_parm_addr_valid(addr, sizeof(IplParameterBlock), write)) {
>           s390_program_interrupt(env, PGM_ADDRESSING, ra);
>           return -1;
>       }

Re: [PATCH v6 06/28] s390x/diag: Refactor address validation check from diag308_parm_check

Posted by Farhan Ali 1 month, 3 weeks ago

Reviewed-by: Farhan Ali <alifm@linux.ibm.com>

On 9/17/2025 4:21 PM, Zhuoying Cai wrote:
> Create a function to validate the address parameter of DIAGNOSE.
>
> Refactor the function for reuse in the next patch, which allows address
> validation in read or write operation of DIAGNOSE.
>
> Signed-off-by: Zhuoying Cai <zycai@linux.ibm.com>
> ---
>   hw/s390x/ipl.h      | 6 ++++++
>   target/s390x/diag.c | 4 +---
>   2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index bee72dfbb3..e26fc1cd6a 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -118,6 +118,12 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb wrong");
>   #define S390_IPLB_MIN_FCP_LEN 384
>   #define S390_IPLB_MIN_QEMU_SCSI_LEN 200
>   
> +static inline bool diag_parm_addr_valid(uint64_t addr, size_t size, bool write)
> +{
> +    return address_space_access_valid(&address_space_memory, addr,
> +                                      size, write, MEMTXATTRS_UNSPECIFIED);
> +}
> +
>   static inline bool iplb_valid_len(IplParameterBlock *iplb)
>   {
>       return be32_to_cpu(iplb->len) <= sizeof(IplParameterBlock);
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index a35d808fd7..e67ee57f01 100644
> --- a/target/s390x/diag.c
> +++ b/target/s390x/diag.c
> @@ -65,9 +65,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
>           s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>           return -1;
>       }
> -    if (!address_space_access_valid(&address_space_memory, addr,
> -                                    sizeof(IplParameterBlock), write,
> -                                    MEMTXATTRS_UNSPECIFIED)) {
> +    if (!diag_parm_addr_valid(addr, sizeof(IplParameterBlock), write)) {
>           s390_program_interrupt(env, PGM_ADDRESSING, ra);
>           return -1;
>       }