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