| Message ID | 1468244718-3731-5-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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); }
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(+)