All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.