All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Tamas K Lengyel" <tamas@tklengyel.com>,
	"Alexandru Isaila" <aisaila@bitdefender.com>,
	"Petre Pircalabu" <ppircalabu@bitdefender.com>
Subject: Re: [PATCH] xen: vm_event: do not do vm_event_op for an invalid domain
Date: Mon, 17 Mar 2025 23:40:59 +0000	[thread overview]
Message-ID: <497de03e-4dbb-4025-806f-46274d7a7c19@citrix.com> (raw)
In-Reply-To: <20250317230806.1179478-1-volodymyr_babchuk@epam.com>

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


  reply	other threads:[~2025-03-17 23:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-17 23:08 [PATCH] xen: vm_event: do not do vm_event_op for an invalid domain Volodymyr Babchuk
2025-03-17 23:40 ` Andrew Cooper [this message]
2025-03-17 23:51 ` Tamas K Lengyel
2025-03-18  0:01   ` Volodymyr Babchuk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=497de03e-4dbb-4025-806f-46274d7a7c19@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=aisaila@bitdefender.com \
    --cc=ppircalabu@bitdefender.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tamas@tklengyel.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.