From e7904f474ef824ba1419c4cf734058f322ffd09e Mon Sep 17 00:00:00 2001 From: Collin Walling Date: Tue, 15 Sep 2020 15:44:11 -0400 Subject: [PATCH 3/9] s390/sclp: read sccb from mem based on provided length The header contained within the SCCB passed to the SCLP service call contains the actual length of the SCCB. Instead of allocating a static 4K size for the work sccb, let's allow for a variable size determined by the value in the header. The proper checks are already in place to ensure the SCCB length is sufficent to store a full response and that the length does not cross any explicitly-set boundaries. Signed-off-by: Collin Walling Reviewed-by: Thomas Huth Reviewed-by: Claudio Imbrenda Message-Id: <20200915194416.107460-4-walling@linux.ibm.com> Signed-off-by: Cornelia Huck (backported from commit c1db53a5910f988eeb32f031c53a50f3373fd824) Signed-off-by: Collin Walling --- hw/s390x/event-facility.c | 2 +- hw/s390x/sclp.c | 57 ++++++++++++++++++++++----------------- include/hw/s390x/sclp.h | 2 +- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c index 66205697ae..8aa7017f06 100644 --- a/hw/s390x/event-facility.c +++ b/hw/s390x/event-facility.c @@ -215,7 +215,7 @@ static uint16_t handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb, event_buf = &red->ebh; event_buf->length = 0; - slen = sizeof(sccb->data); + slen = sccb_data_len(sccb); rc = SCLP_RC_NO_EVENT_BUFFERS_STORED; diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c index ff2734920a..a57c94f201 100644 --- a/hw/s390x/sclp.c +++ b/hw/s390x/sclp.c @@ -231,25 +231,29 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, { SCLPDevice *sclp = get_sclp_device(); SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp); - SCCB work_sccb; - hwaddr sccb_len = sizeof(SCCB); + SCCBHeader header; + g_autofree SCCB *work_sccb = NULL; - s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len); + s390_cpu_pv_mem_read(env_archcpu(env), 0, &header, sizeof(SCCBHeader)); + + work_sccb = g_malloc0(be16_to_cpu(header.length)); + s390_cpu_pv_mem_read(env_archcpu(env), 0, work_sccb, + be16_to_cpu(header.length)); if (!sclp_command_code_valid(code)) { - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); + work_sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); goto out_write; } - if (!sccb_verify_boundary(sccb, be16_to_cpu(work_sccb.h.length))) { - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); + if (!sccb_verify_boundary(sccb, be16_to_cpu(work_sccb->h.length))) { + work_sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); goto out_write; } - sclp_c->execute(sclp, &work_sccb, code); + sclp_c->execute(sclp, work_sccb, code); out_write: - s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, - be16_to_cpu(work_sccb.h.length)); + s390_cpu_pv_mem_write(env_archcpu(env), 0, work_sccb, + be16_to_cpu(work_sccb->h.length)); sclp_c->service_interrupt(sclp, SCLP_PV_DUMMY_ADDR); return 0; } @@ -258,10 +262,9 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) { SCLPDevice *sclp = get_sclp_device(); SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp); + SCCBHeader header; + g_autofree SCCB *work_sccb = NULL; int r = 0; - SCCB work_sccb; - - hwaddr sccb_len = sizeof(SCCB); /* first some basic checks on program checks */ if (env->psw.mask & PSW_MASK_PSTATE) { @@ -278,33 +281,37 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) goto out; } + /* the header contains the actual length of the sccb */ + cpu_physical_memory_read(sccb, &header, sizeof(SCCBHeader)); + + /* Valid sccb sizes */ + if (be16_to_cpu(header.length) < sizeof(SCCBHeader)) { + r = -PGM_SPECIFICATION; + goto out; + } + /* * we want to work on a private copy of the sccb, to prevent guests * from playing dirty tricks by modifying the memory content after * the host has checked the values */ - cpu_physical_memory_read(sccb, &work_sccb, sccb_len); - - /* Valid sccb sizes */ - if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) { - r = -PGM_SPECIFICATION; - goto out; - } + work_sccb = g_malloc0(be16_to_cpu(header.length)); + cpu_physical_memory_read(sccb, work_sccb, be16_to_cpu(header.length)); if (!sclp_command_code_valid(code)) { - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); + work_sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); goto out_write; } - if (!sccb_verify_boundary(sccb, be16_to_cpu(work_sccb.h.length))) { - work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); + if (!sccb_verify_boundary(sccb, be16_to_cpu(work_sccb->h.length))) { + work_sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION); goto out_write; } - sclp_c->execute(sclp, &work_sccb, code); + sclp_c->execute(sclp, work_sccb, code); out_write: - cpu_physical_memory_write(sccb, &work_sccb, - be16_to_cpu(work_sccb.h.length)); + cpu_physical_memory_write(sccb, work_sccb, + be16_to_cpu(work_sccb->h.length)); sclp_c->service_interrupt(sclp, sccb); diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h index c0a3faa37d..55f53a4654 100644 --- a/include/hw/s390x/sclp.h +++ b/include/hw/s390x/sclp.h @@ -177,7 +177,7 @@ typedef struct IoaCfgSccb { typedef struct SCCB { SCCBHeader h; - char data[SCCB_DATA_LEN]; + char data[]; } QEMU_PACKED SCCB; #define TYPE_SCLP "sclp" -- 2.31.1