[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 04/15] nvdimm: support nvdimm label

From: Xiao Guangrong
Subject: Re: [Qemu-devel] [PATCH 04/15] nvdimm: support nvdimm label
Date: Wed, 23 Mar 2016 11:40:57 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1

On 03/17/2016 06:28 PM, Stefan Hajnoczi wrote:
On Thu, Mar 17, 2016 at 04:32:50PM +0800, Xiao Guangrong wrote:
+static void nvdimm_init(Object *obj)
+    object_property_add_bool(obj, "reserve-label", nvdimm_get_reserve_label,
+                             nvdimm_set_reserve_label, NULL);

In the future users may wish for larger namespace label sizes.  This
bool option will not allow that.

Perhaps the option should be an integer called "label-size"?

Yes, good to me.

+static void nvdimm_assert_rw_label_data(NVDIMMDevice *nvdimm, uint64_t size,
+                                        uint64_t offset)
+    assert(nvdimm->reserve_label &&
+           (nvdimm->label_size >= size + offset) && (offset + size > offset));

It's not clear from this patch alone, but QEMU is not allowed to assert
due to invalid inputs from the guest.  So if input validation is
necessary here because the values may be invalid, please write if
statements and error returns.

The caller should check it before calling these callbacks, in our case, we did
it in nvdimm_rw_label_data_check() in patch 13.

So if that happen, it is really a QEMU internal BUG.

This is important so guests cannot cause QEMU to core dump (SIGABRT
default behavior) and so that nested virtualization doesn't allow a
nested guest to DoS its parent guest.

Yes, i understood it, but it is not the case in this patch as the assert()
can not be triggered by guest.

Maybe i should mention it in the changelog to make this fact more clean.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]