Re: [PATCH] hyperv: add check for NULL for msg

["Alex Bennée" <alex.bennee@linaro.org>]

Anastasia Belova <abelova@astralinux.ru> 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 <abelova@astralinux.ru>
> ---
>  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.

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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro