Re: [PATCH] xen: vm_event: do not do vm_event_op for an invalid domain

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/03/2025 11:08 pm, Volodymyr Babchuk wrote:
> A privileged domain can issue XEN_DOMCTL_vm_event_op with
> op->domain == DOMID_INVALID. In this case vm_event_domctl()
> function will get NULL as the first parameter and this will
> cause hypervisor panic, as it tries to derefer this pointer.
>
> Fix the issue by checking if valid domain is passed in.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
>
> ---
>
> This issue was found by the xen fuzzer ([1])
>
> [1] 
> https://lore.kernel.org/all/20250315003544.1101488-1-volodymyr_babchuk@xxxxxxxx/
> ---
>  xen/common/vm_event.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
> index fbf1aa0848..a4c233de52 100644
> --- a/xen/common/vm_event.c
> +++ b/xen/common/vm_event.c
> @@ -600,6 +600,13 @@ int vm_event_domctl(struct domain *d, struct 
> xen_domctl_vm_event_op *vec)
>          return 0;
>      }
>  
> +    if ( unlikely(!d) )
> +    {
> +        gdprintk(XENLOG_INFO,
> +                 "Tried to do a memory event op on invalid domain\n");
> +        return -EINVAL;
> +    }
> +
>      rc = xsm_vm_event_control(XSM_PRIV, d, vec->mode, vec->op);
>      if ( rc )
>          return rc;

Oops.  Git blame says this is my fault.

This behaviour is intentional.  See XEN_DOMCTL_vm_event_op (along with
test_assign_device and get_domain_state) early in do_domctl().

It was introduced in commit d48e1836074c ("vm_event: Add a new opcode to
get VM_EVENT_INTERFACE_VERSION") so that XEN_VM_EVENT_GET_VERSION could
succeed.

Apparently I deleted it in commit 48b84249459f ("xen/vm-event: Drop
unused u_domctl parameter from vm_event_domctl()"), and that wasn't
intentional.

That will want putting in with an extra comment.

/* All other subops need to target a real domain. */
if ( unlikely(d == NULL) )
    return -ESRCH;

Don't both with a printk().  It's just noise, and -ESRCH is correct code
to use.

~Andrew