diff mbox

[4/8] nvdimm acpi: implement Read FIT function

Message ID 1468244718-3731-5-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiao Guangrong July 11, 2016, 1:45 p.m. UTC
Read FIT whose function index is 0xFFFFFFFF is reserved by QEMU to read
the piece of FIT buffer. Please refer to docs/specs/acpi_nvdimm.txt for
detailed info

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/nvdimm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

Comments

Stefan Hajnoczi July 14, 2016, 12:17 p.m. UTC | #1
On Mon, Jul 11, 2016 at 09:45:14PM +0800, Xiao Guangrong wrote:
> Read FIT whose function index is 0xFFFFFFFF is reserved by QEMU to read
> the piece of FIT buffer. Please refer to docs/specs/acpi_nvdimm.txt for
> detailed info
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  hw/acpi/nvdimm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 4bbd1e7..d099ef1 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -466,6 +466,22 @@ typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
>  QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) +
>                    offsetof(NvdimmDsmIn, arg3) > 4096);
>  
> +struct NvdimmFuncReadFITIn {
> +    uint32_t offset; /* the offset of FIT buffer. */
> +} QEMU_PACKED;
> +typedef struct NvdimmFuncReadFITIn NvdimmFuncReadFITIn;
> +QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITIn) +
> +                  offsetof(NvdimmDsmIn, arg3) > 4096);
> +
> +struct NvdimmFuncReadFITOut {
> +    /* the size of buffer filled by QEMU. */
> +    uint32_t len;
> +    uint32_t func_ret_status; /* return status code. */
> +    uint8_t fit[0]; /* the FIT data. */
> +} QEMU_PACKED;
> +typedef struct NvdimmFuncReadFITOut NvdimmFuncReadFITOut;
> +QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITOut) > 4096);
> +
>  static void
>  nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
>  {
> @@ -486,6 +502,46 @@ nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
>      cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
>  }
>  
> +/* Read FIT data, defined in docs/specs/acpi_nvdimm.txt. */
> +static void nvdimm_dsm_func_read_fit(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
> +{
> +    NvdimmFuncReadFITIn *read_fit;
> +    NvdimmFuncReadFITOut *read_fit_out;
> +    GSList *device_list = nvdimm_get_plugged_device_list();
> +    GArray *fit = nvdimm_build_device_structure(device_list);
> +    uint32_t read_len = 0, func_ret_status;
> +    int left, size;
> +
> +    read_fit = (NvdimmFuncReadFITIn *)in->arg3;
> +    le32_to_cpus(&read_fit->offset);
> +
> +    nvdimm_debug("Read FIT: offset %#x FIT size %#x.\n", read_fit->offset,
> +                 fit->len);
> +
> +    left = fit->len - read_fit->offset;
> +    if (left < 0) {

Signed integer overflow leads to memory disclosure in memcpy() below.
The problem occurs when (guint)fit->len - (uint32_t)read_fit->offset >
INT_MAX.

Please perform the check like this:

  if (fit->offset >= fit->len) {

> +        func_ret_status = 3 /* Invalid Input Parameters */;
> +        goto build_out;
> +    }
> +
> +    func_ret_status = 0 /* Success */;
> +    read_len = MIN(left, 4096 - sizeof(NvdimmFuncReadFITOut));
> +
> +build_out:
> +    size = sizeof(NvdimmFuncReadFITOut) + read_len;
> +    read_fit_out = g_malloc(size);
> +
> +    read_fit_out->len = cpu_to_le32(size);
> +    read_fit_out->func_ret_status = cpu_to_le32(func_ret_status);
> +    memcpy(read_fit_out->fit, fit->data + read_fit->offset, read_len);
> +
> +    cpu_physical_memory_write(dsm_mem_addr, read_fit_out, size);
> +
> +    g_slist_free(device_list);
> +    g_array_free(fit, true);
> +    g_free(read_fit_out);
> +}
> +
>  static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
>  {
>      /*
> @@ -498,6 +554,11 @@ static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
>          return;
>      }
>  
> +    if (in->function == 0xFFFFFFFF /* Read FIT */) {
> +        nvdimm_dsm_func_read_fit(in, dsm_mem_addr);
> +        return;
> +    }
> +
>      /* No function except function 0 is supported yet. */
>      nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
>  }
> -- 
> 1.8.3.1
>
Xiao Guangrong July 15, 2016, 7:43 a.m. UTC | #2
On 07/14/2016 08:17 PM, Stefan Hajnoczi wrote:

>> +/* Read FIT data, defined in docs/specs/acpi_nvdimm.txt. */
>> +static void nvdimm_dsm_func_read_fit(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
>> +{
>> +    NvdimmFuncReadFITIn *read_fit;
>> +    NvdimmFuncReadFITOut *read_fit_out;
>> +    GSList *device_list = nvdimm_get_plugged_device_list();
>> +    GArray *fit = nvdimm_build_device_structure(device_list);
>> +    uint32_t read_len = 0, func_ret_status;
>> +    int left, size;
>> +
>> +    read_fit = (NvdimmFuncReadFITIn *)in->arg3;
>> +    le32_to_cpus(&read_fit->offset);
>> +
>> +    nvdimm_debug("Read FIT: offset %#x FIT size %#x.\n", read_fit->offset,
>> +                 fit->len);
>> +
>> +    left = fit->len - read_fit->offset;
>> +    if (left < 0) {
>
> Signed integer overflow leads to memory disclosure in memcpy() below.
> The problem occurs when (guint)fit->len - (uint32_t)read_fit->offset >
> INT_MAX.
>
> Please perform the check like this:
>
>    if (fit->offset >= fit->len) {
>

Ah, yes, you are right, thank you for pointing it out. Will fix it.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 4bbd1e7..d099ef1 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -466,6 +466,22 @@  typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
 QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) +
                   offsetof(NvdimmDsmIn, arg3) > 4096);
 
+struct NvdimmFuncReadFITIn {
+    uint32_t offset; /* the offset of FIT buffer. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncReadFITIn NvdimmFuncReadFITIn;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITIn) +
+                  offsetof(NvdimmDsmIn, arg3) > 4096);
+
+struct NvdimmFuncReadFITOut {
+    /* the size of buffer filled by QEMU. */
+    uint32_t len;
+    uint32_t func_ret_status; /* return status code. */
+    uint8_t fit[0]; /* the FIT data. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncReadFITOut NvdimmFuncReadFITOut;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITOut) > 4096);
+
 static void
 nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
 {
@@ -486,6 +502,46 @@  nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
     cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
 }
 
+/* Read FIT data, defined in docs/specs/acpi_nvdimm.txt. */
+static void nvdimm_dsm_func_read_fit(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    NvdimmFuncReadFITIn *read_fit;
+    NvdimmFuncReadFITOut *read_fit_out;
+    GSList *device_list = nvdimm_get_plugged_device_list();
+    GArray *fit = nvdimm_build_device_structure(device_list);
+    uint32_t read_len = 0, func_ret_status;
+    int left, size;
+
+    read_fit = (NvdimmFuncReadFITIn *)in->arg3;
+    le32_to_cpus(&read_fit->offset);
+
+    nvdimm_debug("Read FIT: offset %#x FIT size %#x.\n", read_fit->offset,
+                 fit->len);
+
+    left = fit->len - read_fit->offset;
+    if (left < 0) {
+        func_ret_status = 3 /* Invalid Input Parameters */;
+        goto build_out;
+    }
+
+    func_ret_status = 0 /* Success */;
+    read_len = MIN(left, 4096 - sizeof(NvdimmFuncReadFITOut));
+
+build_out:
+    size = sizeof(NvdimmFuncReadFITOut) + read_len;
+    read_fit_out = g_malloc(size);
+
+    read_fit_out->len = cpu_to_le32(size);
+    read_fit_out->func_ret_status = cpu_to_le32(func_ret_status);
+    memcpy(read_fit_out->fit, fit->data + read_fit->offset, read_len);
+
+    cpu_physical_memory_write(dsm_mem_addr, read_fit_out, size);
+
+    g_slist_free(device_list);
+    g_array_free(fit, true);
+    g_free(read_fit_out);
+}
+
 static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
     /*
@@ -498,6 +554,11 @@  static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
         return;
     }
 
+    if (in->function == 0xFFFFFFFF /* Read FIT */) {
+        nvdimm_dsm_func_read_fit(in, dsm_mem_addr);
+        return;
+    }
+
     /* No function except function 0 is supported yet. */
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
 }