On 9/11/20 6:24 AM, Cornelia Huck wrote: > On Thu, 10 Sep 2020 19:45:01 +0200 > Thomas Huth <th...@redhat.com> wrote: > >> On 10/09/2020 11.36, Collin Walling wrote: >>> Rework the SCLP boundary check to account for different SCLP commands >>> (eventually) allowing different boundary sizes. >>> >>> Signed-off-by: Collin Walling <wall...@linux.ibm.com> >>> Acked-by: Janosch Frank <fran...@linux.ibm.com> >>> Reviewed-by: Cornelia Huck <coh...@redhat.com> >>> --- >>> hw/s390x/sclp.c | 19 ++++++++++++++++++- >>> 1 file changed, 18 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >>> index 28b973de8f..69a8724dc7 100644 >>> --- a/hw/s390x/sclp.c >>> +++ b/hw/s390x/sclp.c >>> @@ -49,6 +49,18 @@ static inline bool sclp_command_code_valid(uint32_t code) >>> return false; >>> } >>> >>> +static bool sccb_verify_boundary(uint64_t sccb_addr, uint16_t len) >> >> Maybe it would be good to add a comment in front of the function to say >> that len must be big endian? > > What about renaming it to sccb_h_len or so? That would make it more > clear that the parameter is not just some random length. >
I think that makes sense. >> >> Thomas >> >>> +{ >>> + uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(len) - 1; >>> + uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE; >>> + >>> + if (sccb_max_addr < sccb_boundary) { >>> + return true; >>> + } >>> + >>> + return false; >>> +} >>> + >>> static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int >>> *count) >>> { >>> uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 }; >>> @@ -229,6 +241,11 @@ int sclp_service_call_protected(CPUS390XState *env, >>> uint64_t sccb, >>> goto out_write; >>> } >>> >>> + if (!sccb_verify_boundary(sccb, work_sccb.h.length)) { > > ...name inspired by the 'h' in 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: >>> s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, >>> @@ -274,7 +291,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)) { >>> + if (!sccb_verify_boundary(sccb, work_sccb.h.length)) { >>> work_sccb.h.response_code = >>> cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); >>> goto out_write; >>> } >>> >> > > -- Regards, Collin Stay safe and stay healthy