diff mbox

acpi: fix assert failure caused by commit 35c5a52d

Message ID 20161019091925.20446-1-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Oct. 19, 2016, 9:19 a.m. UTC 
Commit 35c5a52d "acpi: do not use TARGET_PAGE_SIZE" changed struct
NvdimmDsmIn from a variable-size structure to a fixed-size structure of
4096 bytes. It forgot to adjust an assert in
nvdimm_dsm_set_label_data(..., NvdimmDsmIn *in, ...):
    assert(sizeof(*in) + sizeof(*set_label_data) + set_label_data->length <=
           4096);
which could crash QEMU when guest writes NVDIMM labels.

Fix it by replacing sizeof(*in) by offsetof(NvdimmDsmIn, arg3).

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
Reported-by: Dan Williams <dan.j.williams@intel.com>
---
 hw/acpi/nvdimm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dan Williams Oct. 19, 2016, 3:50 p.m. UTC | #1 
On Wed, Oct 19, 2016 at 2:19 AM, Haozhong Zhang
<haozhong.zhang@intel.com> wrote:
> Commit 35c5a52d "acpi: do not use TARGET_PAGE_SIZE" changed struct
> NvdimmDsmIn from a variable-size structure to a fixed-size structure of
> 4096 bytes. It forgot to adjust an assert in
> nvdimm_dsm_set_label_data(..., NvdimmDsmIn *in, ...):
>     assert(sizeof(*in) + sizeof(*set_label_data) + set_label_data->length <=
>            4096);
> which could crash QEMU when guest writes NVDIMM labels.
>
> Fix it by replacing sizeof(*in) by offsetof(NvdimmDsmIn, arg3).
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> Reported-by: Dan Williams <dan.j.williams@intel.com>

Thanks!

Tested-by: Dan Williams <dan.j.williams@intel.com>
Xiao Guangrong Oct. 24, 2016, 9:46 a.m. UTC | #2 
On 10/19/2016 05:19 PM, Haozhong Zhang wrote:
> Commit 35c5a52d "acpi: do not use TARGET_PAGE_SIZE" changed struct
> NvdimmDsmIn from a variable-size structure to a fixed-size structure of
> 4096 bytes. It forgot to adjust an assert in
> nvdimm_dsm_set_label_data(..., NvdimmDsmIn *in, ...):
>     assert(sizeof(*in) + sizeof(*set_label_data) + set_label_data->length <=
>            4096);
> which could crash QEMU when guest writes NVDIMM labels.
>
> Fix it by replacing sizeof(*in) by offsetof(NvdimmDsmIn, arg3).

Thanks for your fix.

Reviewed-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
diff mbox

Patch

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e486128..9fdc56a 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -643,8 +643,8 @@  static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
         return;
     }
 
-    assert(sizeof(*in) + sizeof(*set_label_data) + set_label_data->length <=
-           4096);
+    assert(offsetof(NvdimmDsmIn, arg3) +
+           sizeof(*set_label_data) + set_label_data->length <= 4096);
 
     nvc->write_label_data(nvdimm, set_label_data->in_buf,
                           set_label_data->length, set_label_data->offset);