| Message ID | 20200618222258.23287-4-walling@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390: Extended-Length SCCB & DIAGNOSE 0x318 | expand |
On 6/19/20 12:22 AM, Collin Walling wrote: > Rework the SCLP boundary check to account for different SCLP commands > (eventually) allowing different boundary sizes. > > Move the length check code into a separate function, and introduce a > new function to determine the length of the read SCP data (i.e. the size > from the start of the struct to where the CPU entries should begin). > > The format of read CPU info is unlikely to change in the future, > so we do not require a separate function to calculate its length. > > Signed-off-by: Collin Walling <walling@linux.ibm.com> Acked-by: Janosch Frank <frankja@linux.ibm.com> > --- [...] > +/* > + * The data length denotes the start of the struct to where the first > + * CPU entry is to be allocated. This value also denotes the offset_cpu > + * field. > + */ > +static inline int get_read_scp_info_data_len(void) > +{ > + return offsetof(ReadInfo, entries); > +} > + > /* Provide information about the configuration, CPUs and storage */ > static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > { > @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > int cpu_count; > int rnsize, rnmax; > IplParameterBlock *ipib = s390_ipl_get_iplb(); > + int data_len = get_read_scp_info_data_len(); Does somebody have a better name than data_len at hand? > > - if (be16_to_cpu(sccb->h.length) < > - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { > - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); > + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { > return; > } > > /* CPU information */ > prepare_cpu_entries(machine, read_info->entries, &cpu_count); > read_info->entries_cpu = cpu_to_be16(cpu_count); > - read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries)); > + read_info->offset_cpu = cpu_to_be16(data_len); > read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1); > > read_info->ibc_val = cpu_to_be32(s390_get_ibc_val()); > @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb) > { > MachineState *machine = MACHINE(qdev_get_machine()); > ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb; > + int data_len = offsetof(ReadCpuInfo, entries); > int cpu_count; > > - if (be16_to_cpu(sccb->h.length) < > - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { > - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); > + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { > return; > } > > prepare_cpu_entries(machine, cpu_info->entries, &cpu_count); > cpu_info->nr_configured = cpu_to_be16(cpu_count); > - cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries)); > + cpu_info->offset_configured = cpu_to_be16(data_len); > cpu_info->nr_standby = cpu_to_be16(0); > > /* The standby offset is 16-byte for each CPU */ > @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, > goto out_write; > } > > + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { > + goto out_write; > + } > + > sclp_c->execute(sclp, &work_sccb, code); > out_write: > s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, > @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) > goto out_write; > } > > - if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) { > - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); > + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { > goto out_write; > } > >
On Fri, 19 Jun 2020 12:50:11 +0200 Janosch Frank <frankja@linux.ibm.com> wrote: > On 6/19/20 12:22 AM, Collin Walling wrote: > > Rework the SCLP boundary check to account for different SCLP commands > > (eventually) allowing different boundary sizes. > > > > Move the length check code into a separate function, and introduce a > > new function to determine the length of the read SCP data (i.e. the size > > from the start of the struct to where the CPU entries should begin). > > > > The format of read CPU info is unlikely to change in the future, > > so we do not require a separate function to calculate its length. > > > > Signed-off-by: Collin Walling <walling@linux.ibm.com> > > Acked-by: Janosch Frank <frankja@linux.ibm.com> > > > --- > [...] > > +/* > > + * The data length denotes the start of the struct to where the first > > + * CPU entry is to be allocated. This value also denotes the offset_cpu > > + * field. > > + */ > > +static inline int get_read_scp_info_data_len(void) > > +{ > > + return offsetof(ReadInfo, entries); > > +} > > + > > /* Provide information about the configuration, CPUs and storage */ > > static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > > { > > @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > > int cpu_count; > > int rnsize, rnmax; > > IplParameterBlock *ipib = s390_ipl_get_iplb(); > > + int data_len = get_read_scp_info_data_len(); > > Does somebody have a better name than data_len at hand? I could not come up with anything better, either :( (...) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 19.06.20 12:50, Janosch Frank wrote: > On 6/19/20 12:22 AM, Collin Walling wrote: >> Rework the SCLP boundary check to account for different SCLP commands >> (eventually) allowing different boundary sizes. >> >> Move the length check code into a separate function, and introduce a >> new function to determine the length of the read SCP data (i.e. the size >> from the start of the struct to where the CPU entries should begin). >> >> The format of read CPU info is unlikely to change in the future, >> so we do not require a separate function to calculate its length. >> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> > > Acked-by: Janosch Frank <frankja@linux.ibm.com> > >> --- > [...] >> +/* >> + * The data length denotes the start of the struct to where the first >> + * CPU entry is to be allocated. This value also denotes the offset_cpu >> + * field. >> + */ >> +static inline int get_read_scp_info_data_len(void) >> +{ >> + return offsetof(ReadInfo, entries); >> +} >> + >> /* Provide information about the configuration, CPUs and storage */ >> static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >> { >> @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >> int cpu_count; >> int rnsize, rnmax; >> IplParameterBlock *ipib = s390_ipl_get_iplb(); >> + int data_len = get_read_scp_info_data_len(); > > Does somebody have a better name than data_len at hand? I stumbled over the same (also for the function). What about cpu_offset instead of data_len?
On 19.06.20 00:22, Collin Walling wrote: > Rework the SCLP boundary check to account for different SCLP commands > (eventually) allowing different boundary sizes. > > Move the length check code into a separate function, and introduce a > new function to determine the length of the read SCP data (i.e. the size > from the start of the struct to where the CPU entries should begin). > > The format of read CPU info is unlikely to change in the future, > so we do not require a separate function to calculate its length. > > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > hw/s390x/sclp.c | 59 ++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 49 insertions(+), 10 deletions(-) > > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c > index 181ce04007..0710138f91 100644 > --- a/hw/s390x/sclp.c > +++ b/hw/s390x/sclp.c > @@ -49,6 +49,34 @@ static inline bool sclp_command_code_valid(uint32_t code) > return false; > } > > +static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code, > + SCCBHeader *header) As you write to the sccb in case of error, mabye sccb_verify_boundary instead of has_valid. has_valid feels like a read-only function. > +{ > + uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(header->length) - 1; > + uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE; > + > + switch (code & SCLP_CMD_CODE_MASK) { > + default: > + if (sccb_max_addr < sccb_boundary) { > + return true; > + } > + } > + header->response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); > + return false; > +} > + > +/* Calculates sufficient SCCB length to store a full Read SCP/CPU response */ > +static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len) same here, maybe sccb_verify_length > +{ > + int required_len = data_len + num_cpus * sizeof(CPUEntry); > + > + if (be16_to_cpu(sccb->h.length) < required_len) { > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); > + return false; > + } > + return true; > +} > + > static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) > { > uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 }; > @@ -66,6 +94,16 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) > } > } > > +/* > + * The data length denotes the start of the struct to where the first > + * CPU entry is to be allocated. This value also denotes the offset_cpu > + * field. > + */ > +static inline int get_read_scp_info_data_len(void) > +{ > + return offsetof(ReadInfo, entries); > +} > + > /* Provide information about the configuration, CPUs and storage */ > static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > { > @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) > int cpu_count; > int rnsize, rnmax; > IplParameterBlock *ipib = s390_ipl_get_iplb(); > + int data_len = get_read_scp_info_data_len(); > > - if (be16_to_cpu(sccb->h.length) < > - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { > - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); > + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { > return; > } > > /* CPU information */ > prepare_cpu_entries(machine, read_info->entries, &cpu_count); > read_info->entries_cpu = cpu_to_be16(cpu_count); > - read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries)); > + read_info->offset_cpu = cpu_to_be16(data_len); > read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1); > > read_info->ibc_val = cpu_to_be32(s390_get_ibc_val()); > @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb) > { > MachineState *machine = MACHINE(qdev_get_machine()); > ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb; > + int data_len = offsetof(ReadCpuInfo, entries); > int cpu_count; > > - if (be16_to_cpu(sccb->h.length) < > - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { > - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); > + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { > return; > } > > prepare_cpu_entries(machine, cpu_info->entries, &cpu_count); > cpu_info->nr_configured = cpu_to_be16(cpu_count); > - cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries)); > + cpu_info->offset_configured = cpu_to_be16(data_len); > cpu_info->nr_standby = cpu_to_be16(0); > > /* The standby offset is 16-byte for each CPU */ > @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, > goto out_write; > } > > + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { > + goto out_write; > + } > + > sclp_c->execute(sclp, &work_sccb, code); > out_write: > s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, > @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) > goto out_write; > } > > - if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) { > - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); > + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { > goto out_write; > } > >
On 6/22/20 11:22 AM, Christian Borntraeger wrote: > > > On 19.06.20 00:22, Collin Walling wrote: >> Rework the SCLP boundary check to account for different SCLP commands >> (eventually) allowing different boundary sizes. >> >> Move the length check code into a separate function, and introduce a >> new function to determine the length of the read SCP data (i.e. the size >> from the start of the struct to where the CPU entries should begin). >> >> The format of read CPU info is unlikely to change in the future, >> so we do not require a separate function to calculate its length. >> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> --- >> hw/s390x/sclp.c | 59 ++++++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 49 insertions(+), 10 deletions(-) >> >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >> index 181ce04007..0710138f91 100644 >> --- a/hw/s390x/sclp.c >> +++ b/hw/s390x/sclp.c >> @@ -49,6 +49,34 @@ static inline bool sclp_command_code_valid(uint32_t code) >> return false; >> } >> >> +static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code, >> + SCCBHeader *header) > > As you write to the sccb in case of error, mabye > sccb_verify_boundary instead of has_valid. has_valid feels like a read-only function. > >> +{ >> + uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(header->length) - 1; >> + uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE; >> + >> + switch (code & SCLP_CMD_CODE_MASK) { >> + default: >> + if (sccb_max_addr < sccb_boundary) { >> + return true; >> + } >> + } >> + header->response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); >> + return false; >> +} >> + >> +/* Calculates sufficient SCCB length to store a full Read SCP/CPU response */ >> +static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len) > > same here, maybe sccb_verify_length Sounds good. I was struggling with a decent naming scheme for these as well :) > >> +{ >> + int required_len = data_len + num_cpus * sizeof(CPUEntry); >> + >> + if (be16_to_cpu(sccb->h.length) < required_len) { >> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); >> + return false; >> + } >> + return true; >> +} >> + >> static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) >> { >> uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 }; >> @@ -66,6 +94,16 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) >> } >> } >> >> +/* >> + * The data length denotes the start of the struct to where the first >> + * CPU entry is to be allocated. This value also denotes the offset_cpu >> + * field. >> + */ >> +static inline int get_read_scp_info_data_len(void) >> +{ >> + return offsetof(ReadInfo, entries); >> +} >> + >> /* Provide information about the configuration, CPUs and storage */ >> static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >> { >> @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) >> int cpu_count; >> int rnsize, rnmax; >> IplParameterBlock *ipib = s390_ipl_get_iplb(); >> + int data_len = get_read_scp_info_data_len(); >> >> - if (be16_to_cpu(sccb->h.length) < >> - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { >> - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); >> + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { >> return; >> } >> >> /* CPU information */ >> prepare_cpu_entries(machine, read_info->entries, &cpu_count); >> read_info->entries_cpu = cpu_to_be16(cpu_count); >> - read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries)); >> + read_info->offset_cpu = cpu_to_be16(data_len); >> read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1); >> >> read_info->ibc_val = cpu_to_be32(s390_get_ibc_val()); >> @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb) >> { >> MachineState *machine = MACHINE(qdev_get_machine()); >> ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb; >> + int data_len = offsetof(ReadCpuInfo, entries); >> int cpu_count; >> >> - if (be16_to_cpu(sccb->h.length) < >> - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { >> - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); >> + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { >> return; >> } >> >> prepare_cpu_entries(machine, cpu_info->entries, &cpu_count); >> cpu_info->nr_configured = cpu_to_be16(cpu_count); >> - cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries)); >> + cpu_info->offset_configured = cpu_to_be16(data_len); >> cpu_info->nr_standby = cpu_to_be16(0); >> >> /* The standby offset is 16-byte for each CPU */ >> @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, >> goto out_write; >> } >> >> + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { >> + goto out_write; >> + } >> + >> sclp_c->execute(sclp, &work_sccb, code); >> out_write: >> s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, >> @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) >> goto out_write; >> } >> >> - if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) { >> - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); >> + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { >> goto out_write; >> } >> >>
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c index 181ce04007..0710138f91 100644 --- a/hw/s390x/sclp.c +++ b/hw/s390x/sclp.c @@ -49,6 +49,34 @@ static inline bool sclp_command_code_valid(uint32_t code) return false; } +static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code, + SCCBHeader *header) +{ + uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(header->length) - 1; + uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE; + + switch (code & SCLP_CMD_CODE_MASK) { + default: + if (sccb_max_addr < sccb_boundary) { + return true; + } + } + header->response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); + return false; +} + +/* Calculates sufficient SCCB length to store a full Read SCP/CPU response */ +static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len) +{ + int required_len = data_len + num_cpus * sizeof(CPUEntry); + + if (be16_to_cpu(sccb->h.length) < required_len) { + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); + return false; + } + return true; +} + static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) { uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 }; @@ -66,6 +94,16 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count) } } +/* + * The data length denotes the start of the struct to where the first + * CPU entry is to be allocated. This value also denotes the offset_cpu + * field. + */ +static inline int get_read_scp_info_data_len(void) +{ + return offsetof(ReadInfo, entries); +} + /* Provide information about the configuration, CPUs and storage */ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) { @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb) int cpu_count; int rnsize, rnmax; IplParameterBlock *ipib = s390_ipl_get_iplb(); + int data_len = get_read_scp_info_data_len(); - if (be16_to_cpu(sccb->h.length) < - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { return; } /* CPU information */ prepare_cpu_entries(machine, read_info->entries, &cpu_count); read_info->entries_cpu = cpu_to_be16(cpu_count); - read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries)); + read_info->offset_cpu = cpu_to_be16(data_len); read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1); read_info->ibc_val = cpu_to_be32(s390_get_ibc_val()); @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb) { MachineState *machine = MACHINE(qdev_get_machine()); ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb; + int data_len = offsetof(ReadCpuInfo, entries); int cpu_count; - if (be16_to_cpu(sccb->h.length) < - (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) { - sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH); + if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) { return; } prepare_cpu_entries(machine, cpu_info->entries, &cpu_count); cpu_info->nr_configured = cpu_to_be16(cpu_count); - cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries)); + cpu_info->offset_configured = cpu_to_be16(data_len); cpu_info->nr_standby = cpu_to_be16(0); /* The standby offset is 16-byte for each CPU */ @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, goto out_write; } + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { + goto out_write; + } + sclp_c->execute(sclp, &work_sccb, code); out_write: s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) goto out_write; } - if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) { - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); + if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) { goto out_write; }
Rework the SCLP boundary check to account for different SCLP commands (eventually) allowing different boundary sizes. Move the length check code into a separate function, and introduce a new function to determine the length of the read SCP data (i.e. the size from the start of the struct to where the CPU entries should begin). The format of read CPU info is unlikely to change in the future, so we do not require a separate function to calculate its length. Signed-off-by: Collin Walling <walling@linux.ibm.com> --- hw/s390x/sclp.c | 59 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 10 deletions(-)