On Sun, Oct 11, 2015 at 11:52:59AM +0800, Xiao Guangrong wrote:
> static void dsm_write(void *opaque, hwaddr addr,
> uint64_t val, unsigned size)
> {
> + NVDIMMState *state = opaque;
> + MemoryRegion *dsm_ram_mr;
> + dsm_in *in;
> + dsm_out *out;
> + uint32_t revision, function, handle;
> +
> if (val != NOTIFY_VALUE) {
> fprintf(stderr, "BUG: unexepected notify value 0x%" PRIx64, val);
> }
> +
> + dsm_ram_mr = memory_region_find(&state->mr, state->page_size,
> + state->page_size).mr;
> + memory_region_unref(dsm_ram_mr);
> + in = memory_region_get_ram_ptr(dsm_ram_mr);
This looks suspicious. Shouldn't the memory_region_unref(dsm_ram_mr)
happen after we're done using it?
> + out = (dsm_out *)in;
> +
> + revision = in->arg1;
> + function = in->arg2;
> + handle = in->handle;
> + le32_to_cpus(&revision);
> + le32_to_cpus(&function);
> + le32_to_cpus(&handle);
> +
> + nvdebug("UUID " UUID_FMT ".\n", in->arg0[0], in->arg0[1], in->arg0[2],
> + in->arg0[3], in->arg0[4], in->arg0[5], in->arg0[6],
> + in->arg0[7], in->arg0[8], in->arg0[9], in->arg0[10],
> + in->arg0[11], in->arg0[12], in->arg0[13], in->arg0[14],
> + in->arg0[15]);
> + nvdebug("Revision %#x Function %#x Handler %#x.\n", revision, function,
> + handle);
> +
> + if (revision != DSM_REVISION) {
> + nvdebug("Revision %#x is not supported, expect %#x.\n",
> + revision, DSM_REVISION);
> + goto exit;
> + }
> +
> + if (!handle) {
> + if (!dsm_is_root_uuid(in->arg0)) {
Please don't dereference 'in' or pass it to other functions. Avoid race
conditions with guest vcpus by coping in the entire dsm_in struct.
This is like a system call - the kernel cannot trust userspace memory
and must copy in before accessing data. The same rules apply.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
