qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] s390x: SCLP error cleanup
@ 2019-09-26 11:33 Claudio Imbrenda
  2019-09-26 11:33 ` [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority Claudio Imbrenda
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2019-09-26 11:33 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

SCLP doesn't report a lot of errors like it should do, let's fix that.

Janosch Frank (1):
  s390x: Add sclp boundary check and fix error priority

Claudio Imbrenda (1):
  s390x: Fix SCLP return code when buffer too small

 hw/s390x/event-facility.c |  3 ---
 hw/s390x/sclp.c           | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 32 insertions(+), 6 deletions(-)

-- 
2.7.4



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-26 11:33 [PATCH v1 0/2] s390x: SCLP error cleanup Claudio Imbrenda
@ 2019-09-26 11:33 ` Claudio Imbrenda
  2019-09-26 12:21   ` Thomas Huth
  2019-09-27  8:51   ` David Hildenbrand
  2019-09-26 11:33 ` [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small Claudio Imbrenda
  2019-09-27  8:37 ` [PATCH v1 0/2] s390x: SCLP error cleanup David Hildenbrand
  2 siblings, 2 replies; 12+ messages in thread
From: Claudio Imbrenda @ 2019-09-26 11:33 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

From: Janosch Frank <frankja@linux.ibm.com>

* All sclp codes need to be checked for page boundary violations.
* Requests over 4k are not a spec exception.
* Invalid command checking has to be done before the boundary check.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
---
 hw/s390x/event-facility.c |  3 ---
 hw/s390x/sclp.c           | 25 ++++++++++++++++++++++---
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 797ecbb..6620569 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -377,9 +377,6 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
     case SCLP_CMD_WRITE_EVENT_MASK:
         write_event_mask(ef, sccb);
         break;
-    default:
-        sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
-        break;
     }
 }
 
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index fac7c3b..76feac8 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -213,14 +213,33 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
     cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
 
     /* Valid sccb sizes */
-    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader) ||
-        be16_to_cpu(work_sccb.h.length) > SCCB_SIZE) {
+    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) {
         r = -PGM_SPECIFICATION;
         goto out;
     }
 
-    sclp_c->execute(sclp, &work_sccb, code);
+    switch (code & SCLP_CMD_CODE_MASK) {
+    case SCLP_CMDW_READ_SCP_INFO:
+    case SCLP_CMDW_READ_SCP_INFO_FORCED:
+    case SCLP_CMDW_READ_CPU_INFO:
+    case SCLP_CMDW_CONFIGURE_IOA:
+    case SCLP_CMDW_DECONFIGURE_IOA:
+    case SCLP_CMD_READ_EVENT_DATA:
+    case SCLP_CMD_WRITE_EVENT_DATA:
+    case SCLP_CMD_WRITE_EVENT_MASK:
+        break;
+    default:
+        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+        goto out_write;
+    }
 
+    if ((sccb + work_sccb.h.length) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
+        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
+        goto out_write;
+    }
+
+    sclp_c->execute(sclp, &work_sccb, code);
+out_write:
     cpu_physical_memory_write(sccb, &work_sccb,
                               be16_to_cpu(work_sccb.h.length));
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small
  2019-09-26 11:33 [PATCH v1 0/2] s390x: SCLP error cleanup Claudio Imbrenda
  2019-09-26 11:33 ` [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority Claudio Imbrenda
@ 2019-09-26 11:33 ` Claudio Imbrenda
  2019-09-26 12:20   ` Thomas Huth
  2019-09-27  8:37 ` [PATCH v1 0/2] s390x: SCLP error cleanup David Hildenbrand
  2 siblings, 1 reply; 12+ messages in thread
From: Claudio Imbrenda @ 2019-09-26 11:33 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

Return the correct error code when the SCCB buffer is too small to
contain all of the output, for the Read SCP Information and
Read CPU Information commands.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
---
 hw/s390x/sclp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 76feac8..8f7fe1c 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -68,6 +68,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 
     read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
 
+    if (sccb->h.length < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        return;
+    }
+
     /* Configuration Characteristic (Extension) */
     s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
                          read_info->conf_char);
@@ -118,6 +123,11 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
     cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
     cpu_info->nr_standby = cpu_to_be16(0);
 
+    if (sccb->h.length < (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        return;
+    }
+
     /* The standby offset is 16-byte for each CPU */
     cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
         + cpu_info->nr_configured*sizeof(CPUEntry));
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small
  2019-09-26 11:33 ` [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small Claudio Imbrenda
@ 2019-09-26 12:20   ` Thomas Huth
  2019-09-26 12:26     ` Thomas Huth
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Huth @ 2019-09-26 12:20 UTC (permalink / raw)
  To: Claudio Imbrenda, qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

On 26/09/2019 13.33, Claudio Imbrenda wrote:
> Return the correct error code when the SCCB buffer is too small to
> contain all of the output, for the Read SCP Information and
> Read CPU Information commands.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  hw/s390x/sclp.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 76feac8..8f7fe1c 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -68,6 +68,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>  
>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
>  
> +    if (sccb->h.length < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {

Don't you need a cpu16_to_cpu() around sccb->h.length?

> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return;
> +    }
> +
>      /* Configuration Characteristic (Extension) */
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
>                           read_info->conf_char);
> @@ -118,6 +123,11 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>      cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
>      cpu_info->nr_standby = cpu_to_be16(0);
>  
> +    if (sccb->h.length < (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {

dito?

> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return;
> +    }
> +
>      /* The standby offset is 16-byte for each CPU */
>      cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
>          + cpu_info->nr_configured*sizeof(CPUEntry));
> 

 Thomas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-26 11:33 ` [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority Claudio Imbrenda
@ 2019-09-26 12:21   ` Thomas Huth
  2019-09-27  8:51   ` David Hildenbrand
  1 sibling, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2019-09-26 12:21 UTC (permalink / raw)
  To: Claudio Imbrenda, qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

On 26/09/2019 13.33, Claudio Imbrenda wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
> 
> * All sclp codes need to be checked for page boundary violations.
> * Requests over 4k are not a spec exception.
> * Invalid command checking has to be done before the boundary check.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  hw/s390x/event-facility.c |  3 ---
>  hw/s390x/sclp.c           | 25 ++++++++++++++++++++++---
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 797ecbb..6620569 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -377,9 +377,6 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
>      case SCLP_CMD_WRITE_EVENT_MASK:
>          write_event_mask(ef, sccb);
>          break;
> -    default:
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> -        break;
>      }
>  }
>  
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index fac7c3b..76feac8 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -213,14 +213,33 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>      cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
>  
>      /* Valid sccb sizes */
> -    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader) ||
> -        be16_to_cpu(work_sccb.h.length) > SCCB_SIZE) {
> +    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) {
>          r = -PGM_SPECIFICATION;
>          goto out;
>      }
>  
> -    sclp_c->execute(sclp, &work_sccb, code);
> +    switch (code & SCLP_CMD_CODE_MASK) {
> +    case SCLP_CMDW_READ_SCP_INFO:
> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
> +    case SCLP_CMDW_READ_CPU_INFO:
> +    case SCLP_CMDW_CONFIGURE_IOA:
> +    case SCLP_CMDW_DECONFIGURE_IOA:
> +    case SCLP_CMD_READ_EVENT_DATA:
> +    case SCLP_CMD_WRITE_EVENT_DATA:
> +    case SCLP_CMD_WRITE_EVENT_MASK:
> +        break;
> +    default:
> +        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> +        goto out_write;
> +    }
>  
> +    if ((sccb + work_sccb.h.length) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {

I think you likely miss a be16_to_cpu() around work_sccb.h.length here?

> +        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> +        goto out_write;
> +    }
> +
> +    sclp_c->execute(sclp, &work_sccb, code);
> +out_write:
>      cpu_physical_memory_write(sccb, &work_sccb,
>                                be16_to_cpu(work_sccb.h.length));

At least here it is swapped --------^

 Thomas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small
  2019-09-26 12:20   ` Thomas Huth
@ 2019-09-26 12:26     ` Thomas Huth
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2019-09-26 12:26 UTC (permalink / raw)
  To: Claudio Imbrenda, qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

On 26/09/2019 14.20, Thomas Huth wrote:
> On 26/09/2019 13.33, Claudio Imbrenda wrote:
>> Return the correct error code when the SCCB buffer is too small to
>> contain all of the output, for the Read SCP Information and
>> Read CPU Information commands.
>>
>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
>> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>>  hw/s390x/sclp.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 76feac8..8f7fe1c 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -68,6 +68,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>  
>>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
>>  
>> +    if (sccb->h.length < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
> 
> Don't you need a cpu16_to_cpu() around sccb->h.length?

I meant be16_to_cpu(), obviously.

 Thomas


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 0/2] s390x: SCLP error cleanup
  2019-09-26 11:33 [PATCH v1 0/2] s390x: SCLP error cleanup Claudio Imbrenda
  2019-09-26 11:33 ` [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority Claudio Imbrenda
  2019-09-26 11:33 ` [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small Claudio Imbrenda
@ 2019-09-27  8:37 ` David Hildenbrand
  2 siblings, 0 replies; 12+ messages in thread
From: David Hildenbrand @ 2019-09-27  8:37 UTC (permalink / raw)
  To: Claudio Imbrenda, qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

On 26.09.19 13:33, Claudio Imbrenda wrote:
> SCLP doesn't report a lot of errors like it should do, let's fix that.
> 
> Janosch Frank (1):
>   s390x: Add sclp boundary check and fix error priority
> 
> Claudio Imbrenda (1):
>   s390x: Fix SCLP return code when buffer too small
> 
>  hw/s390x/event-facility.c |  3 ---
>  hw/s390x/sclp.c           | 35 ++++++++++++++++++++++++++++++++---
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 

Just wondering, are you using scripts/get_maintainer.pl ? I would have
assume to get CCed on these patches (along with Richard).

-- 

Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-26 11:33 ` [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority Claudio Imbrenda
  2019-09-26 12:21   ` Thomas Huth
@ 2019-09-27  8:51   ` David Hildenbrand
  2019-09-27  9:14     ` Janosch Frank
  1 sibling, 1 reply; 12+ messages in thread
From: David Hildenbrand @ 2019-09-27  8:51 UTC (permalink / raw)
  To: Claudio Imbrenda, qemu-devel, qemu-s390x; +Cc: borntraeger, cohuck, frankja

On 26.09.19 13:33, Claudio Imbrenda wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
> 
> * All sclp codes need to be checked for page boundary violations.
> * Requests over 4k are not a spec exception.
> * Invalid command checking has to be done before the boundary check.

Can we split this patch up so we fix one thing at a time?

> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
> ---
>  hw/s390x/event-facility.c |  3 ---
>  hw/s390x/sclp.c           | 25 ++++++++++++++++++++++---
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 797ecbb..6620569 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -377,9 +377,6 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
>      case SCLP_CMD_WRITE_EVENT_MASK:
>          write_event_mask(ef, sccb);
>          break;
> -    default:
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> -        break;
>      }
>  }
>  
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index fac7c3b..76feac8 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -213,14 +213,33 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>      cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
>  
>      /* Valid sccb sizes */
> -    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader) ||
> -        be16_to_cpu(work_sccb.h.length) > SCCB_SIZE) {
> +    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) {
>          r = -PGM_SPECIFICATION;
>          goto out;
>      }
>  
> -    sclp_c->execute(sclp, &work_sccb, code);
> +    switch (code & SCLP_CMD_CODE_MASK) {
> +    case SCLP_CMDW_READ_SCP_INFO:
> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
> +    case SCLP_CMDW_READ_CPU_INFO:
> +    case SCLP_CMDW_CONFIGURE_IOA:
> +    case SCLP_CMDW_DECONFIGURE_IOA:
> +    case SCLP_CMD_READ_EVENT_DATA:
> +    case SCLP_CMD_WRITE_EVENT_DATA:
> +    case SCLP_CMD_WRITE_EVENT_MASK:
> +        break;
> +    default:
> +        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> +        goto out_write;
> +    }
>  
> +    if ((sccb + work_sccb.h.length) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
> +        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> +        goto out_write;
> +    }
> +
> +    sclp_c->execute(sclp, &work_sccb, code);
> +out_write:
>      cpu_physical_memory_write(sccb, &work_sccb,
>                                be16_to_cpu(work_sccb.h.length));
>  
> 


-- 

Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-27  8:51   ` David Hildenbrand
@ 2019-09-27  9:14     ` Janosch Frank
  2019-09-27  9:17       ` David Hildenbrand
  0 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-09-27  9:14 UTC (permalink / raw)
  To: David Hildenbrand, Claudio Imbrenda, qemu-devel, qemu-s390x
  Cc: borntraeger, cohuck


[-- Attachment #1.1: Type: text/plain, Size: 2913 bytes --]

On 9/27/19 10:51 AM, David Hildenbrand wrote:
> On 26.09.19 13:33, Claudio Imbrenda wrote:
>> From: Janosch Frank <frankja@linux.ibm.com>
>>
>> * All sclp codes need to be checked for page boundary violations.
>> * Requests over 4k are not a spec exception.
>> * Invalid command checking has to be done before the boundary check.
> 
> Can we split this patch up so we fix one thing at a time?

Sure, but we would end up with very small patches.
Do you want that?

> 
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
>> ---
>>  hw/s390x/event-facility.c |  3 ---
>>  hw/s390x/sclp.c           | 25 ++++++++++++++++++++++---
>>  2 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
>> index 797ecbb..6620569 100644
>> --- a/hw/s390x/event-facility.c
>> +++ b/hw/s390x/event-facility.c
>> @@ -377,9 +377,6 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
>>      case SCLP_CMD_WRITE_EVENT_MASK:
>>          write_event_mask(ef, sccb);
>>          break;
>> -    default:
>> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>> -        break;
>>      }
>>  }
>>  
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index fac7c3b..76feac8 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -213,14 +213,33 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>>      cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
>>  
>>      /* Valid sccb sizes */
>> -    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader) ||
>> -        be16_to_cpu(work_sccb.h.length) > SCCB_SIZE) {
>> +    if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) {
>>          r = -PGM_SPECIFICATION;
>>          goto out;
>>      }
>>  
>> -    sclp_c->execute(sclp, &work_sccb, code);
>> +    switch (code & SCLP_CMD_CODE_MASK) {
>> +    case SCLP_CMDW_READ_SCP_INFO:
>> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
>> +    case SCLP_CMDW_READ_CPU_INFO:
>> +    case SCLP_CMDW_CONFIGURE_IOA:
>> +    case SCLP_CMDW_DECONFIGURE_IOA:
>> +    case SCLP_CMD_READ_EVENT_DATA:
>> +    case SCLP_CMD_WRITE_EVENT_DATA:
>> +    case SCLP_CMD_WRITE_EVENT_MASK:
>> +        break;
>> +    default:
>> +        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>> +        goto out_write;
>> +    }
>>  
>> +    if ((sccb + work_sccb.h.length) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
>> +        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>> +        goto out_write;
>> +    }
>> +
>> +    sclp_c->execute(sclp, &work_sccb, code);
>> +out_write:
>>      cpu_physical_memory_write(sccb, &work_sccb,
>>                                be16_to_cpu(work_sccb.h.length));
>>  
>>
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-27  9:14     ` Janosch Frank
@ 2019-09-27  9:17       ` David Hildenbrand
  2019-09-27  9:20         ` Janosch Frank
  0 siblings, 1 reply; 12+ messages in thread
From: David Hildenbrand @ 2019-09-27  9:17 UTC (permalink / raw)
  To: Janosch Frank, Claudio Imbrenda, qemu-devel, qemu-s390x
  Cc: borntraeger, cohuck

On 27.09.19 11:14, Janosch Frank wrote:
> On 9/27/19 10:51 AM, David Hildenbrand wrote:
>> On 26.09.19 13:33, Claudio Imbrenda wrote:
>>> From: Janosch Frank <frankja@linux.ibm.com>
>>>
>>> * All sclp codes need to be checked for page boundary violations.
>>> * Requests over 4k are not a spec exception.
>>> * Invalid command checking has to be done before the boundary check.
>>
>> Can we split this patch up so we fix one thing at a time?
> 
> Sure, but we would end up with very small patches.
> Do you want that?

Why should I say no to easy-to-review, logically consistent, small
chunks? I have shortcuts for my RB's and ACK's, so I don't have to type
much ;)

-- 

Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-27  9:17       ` David Hildenbrand
@ 2019-09-27  9:20         ` Janosch Frank
  2019-09-27  9:29           ` David Hildenbrand
  0 siblings, 1 reply; 12+ messages in thread
From: Janosch Frank @ 2019-09-27  9:20 UTC (permalink / raw)
  To: David Hildenbrand, Claudio Imbrenda, qemu-devel, qemu-s390x
  Cc: borntraeger, cohuck


[-- Attachment #1.1: Type: text/plain, Size: 801 bytes --]

On 9/27/19 11:17 AM, David Hildenbrand wrote:
> On 27.09.19 11:14, Janosch Frank wrote:
>> On 9/27/19 10:51 AM, David Hildenbrand wrote:
>>> On 26.09.19 13:33, Claudio Imbrenda wrote:
>>>> From: Janosch Frank <frankja@linux.ibm.com>
>>>>
>>>> * All sclp codes need to be checked for page boundary violations.
>>>> * Requests over 4k are not a spec exception.
>>>> * Invalid command checking has to be done before the boundary check.
>>>
>>> Can we split this patch up so we fix one thing at a time?
>>
>> Sure, but we would end up with very small patches.
>> Do you want that?
> 
> Why should I say no to easy-to-review, logically consistent, small
> chunks? I have shortcuts for my RB's and ACK's, so I don't have to type
> much ;)
> 

Higher patch count for me, win - win :-)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority
  2019-09-27  9:20         ` Janosch Frank
@ 2019-09-27  9:29           ` David Hildenbrand
  0 siblings, 0 replies; 12+ messages in thread
From: David Hildenbrand @ 2019-09-27  9:29 UTC (permalink / raw)
  To: Janosch Frank, Claudio Imbrenda, qemu-devel, qemu-s390x
  Cc: borntraeger, cohuck

On 27.09.19 11:20, Janosch Frank wrote:
> On 9/27/19 11:17 AM, David Hildenbrand wrote:
>> On 27.09.19 11:14, Janosch Frank wrote:
>>> On 9/27/19 10:51 AM, David Hildenbrand wrote:
>>>> On 26.09.19 13:33, Claudio Imbrenda wrote:
>>>>> From: Janosch Frank <frankja@linux.ibm.com>
>>>>>
>>>>> * All sclp codes need to be checked for page boundary violations.
>>>>> * Requests over 4k are not a spec exception.
>>>>> * Invalid command checking has to be done before the boundary check.
>>>>
>>>> Can we split this patch up so we fix one thing at a time?
>>>
>>> Sure, but we would end up with very small patches.
>>> Do you want that?
>>
>> Why should I say no to easy-to-review, logically consistent, small
>> chunks? I have shortcuts for my RB's and ACK's, so I don't have to type
>> much ;)
>>
> 
> Higher patch count for me, win - win :-)
> 

Now that's the spirit :)

-- 

Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-09-27  9:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-26 11:33 [PATCH v1 0/2] s390x: SCLP error cleanup Claudio Imbrenda
2019-09-26 11:33 ` [PATCH v1 1/2] s390x: Add sclp boundary check and fix error priority Claudio Imbrenda
2019-09-26 12:21   ` Thomas Huth
2019-09-27  8:51   ` David Hildenbrand
2019-09-27  9:14     ` Janosch Frank
2019-09-27  9:17       ` David Hildenbrand
2019-09-27  9:20         ` Janosch Frank
2019-09-27  9:29           ` David Hildenbrand
2019-09-26 11:33 ` [PATCH v1 2/2] s390x: Fix SCLP return code when buffer too small Claudio Imbrenda
2019-09-26 12:20   ` Thomas Huth
2019-09-26 12:26     ` Thomas Huth
2019-09-27  8:37 ` [PATCH v1 0/2] s390x: SCLP error cleanup David Hildenbrand

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).