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 <[email protected]>
>
> ---
>
> This issue was found by the xen fuzzer ([1])
>
> [1]
> https://lore.kernel.org/all/[email protected]/
> ---
> 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
