On 28.09.2023 18:56, Alex Bennée wrote:
Anastasia Belova <[email protected]> writes:cpu_physical_memory_map may return NULL in hyperv_hcall_post_message. Add check for NULL to avoid NULL-dereference. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 76036a5fc7 ("hyperv: process POST_MESSAGE hypercall") Signed-off-by: Anastasia Belova <[email protected]> --- hw/hyperv/hyperv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c index 57b402b956..61c65d7329 100644 --- a/hw/hyperv/hyperv.c +++ b/hw/hyperv/hyperv.c @@ -588,7 +588,7 @@ uint16_t hyperv_hcall_post_message(uint64_t param, bool fast)len = sizeof(*msg);msg = cpu_physical_memory_map(param, &len, 0); - if (len < sizeof(*msg)) { + if (!msg || len < sizeof(*msg)) { ret = HV_STATUS_INSUFFICIENT_MEMORY; goto unmap;What is the failure path that returns NULL but leaves len untouched? I see in address_space_map(): if (!memory_access_is_direct(mr, is_write)) { if (qatomic_xchg(&bounce.in_use, true)) { *plen = 0; return NULL; } and in qemu_ram_ptr_length: if (*size == 0) { return NULL; } but the other paths can't fail AFAICT.
Looks like at least xen_map_cache() can fail with NULL, and this NULL can then be returned from qemu_ram_ptr_length(). In this case, the returned len would come from the return value of flatview_extend_translation() call earlier. To be fair, I am not sure how realistic this scenario is, but most cpu_physical_memory_map() callers seem to check at least its return value, if not return value AND the returned length.
That's not to say its a bad thing to verify the ptr before attempting a de-reference but I would like to understand the failure mode. As an aside it would also be nice to add (as a fresh commit) a kdoc comment for cpu_physical_memory_map() that documents the API.
This would be very nice indeed - to enshrine this function semantics so there aren't future doubts what it can return. Thanks, Maciej
