From 212c129b82f0a53725a4167303de2ee0a865f82d Mon Sep 17 00:00:00 2001

From: Thomas Huth <thuth@redhat.com>

Date: Wed, 11 Nov 2020 12:03:08 -0500

Subject: [PATCH 08/18] s390/sclp: read sccb from mem based on provided length

RH-Author: Thomas Huth <thuth@redhat.com>

Message-id: <20201111120316.707489-5-thuth@redhat.com>

Patchwork-id: 99501

O-Subject: [RHEL-8.4.0 qemu-kvm PATCH v2 04/12] s390/sclp: read sccb from mem based on provided length

Bugzilla: 1798506

RH-Acked-by: Jens Freimann <jfreimann@redhat.com>

RH-Acked-by: Cornelia Huck <cohuck@redhat.com>

RH-Acked-by: David Hildenbrand <david@redhat.com>

From: Collin Walling <walling@linux.ibm.com>

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 <walling@linux.ibm.com>

Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Message-Id: <20200915194416.107460-4-walling@linux.ibm.com>

Signed-off-by: Cornelia Huck <cohuck@redhat.com>

(cherry picked from commit c1db53a5910f988eeb32f031c53a50f3373fd824)

Signed-off-by: Thomas Huth <thuth@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

---

 hw/s390x/event-facility.c |  2 +-

 hw/s390x/sclp.c           | 55 ++++++++++++++++++++++-----------------

 include/hw/s390x/sclp.h   |  2 +-

 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c

index 66205697ae7..8aa7017f06b 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 38278497319..cf1292beb22 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,9 +262,8 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)

 {

     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;

     /* first some basic checks on program checks */

     if (env->psw.mask & PSW_MASK_PSTATE) {

@@ -274,32 +277,36 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)

         return -PGM_SPECIFICATION;

     }

+    /* 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)) {

+        return -PGM_SPECIFICATION;

+    }

+

     /*

      * 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)) {

-        return -PGM_SPECIFICATION;

-    }

+    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 c0a3faa37d7..55f53a46540 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.27.0