[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-4.22? 7/9] domctl: restrict permission check for XEN_DOMCTL_memory_mapping's remove form
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Tue, 16 Jun 2026 11:08:03 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=iM6TSo4/zzYWieftzZTIFXzLwVw4Q7MsnYwayOY/Fn4=; b=wd0qsZKeuG7BkrcdfWbJUv+fmAZtmqHRwz1gmVrY0Pfov23W8/JwfxwepB84W19Vc5UpXrGbYPSAVUR8iwcrDNsCC1nwi25XnACIlV41PR96AwIQ+5B8DH7RwzaxIsH6t1yhoyMlncyTHZhNe8UGK8zpeP2DsUO+9fXoAPutQSJAHamQfUoZNDga4/FwHw6nK8zbtDExlhvVM6ERf2Wax4rnYKMU4Ngyq6lw3PYcXCP7ydkf9LCK0KPcnU/jxYUGCIYsVydW1Ixwl/fAfMz+9YQhZQgYH15v2neAsYjqJZgG220xGSox6ItXqvvCG1JjweC4HnsEvySIR9Y6u8MMkg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=e8kFJaeyZ27gqjRszIOB7h5aG1XF/UhddBm7uhhiVjo/8oNci80B6NLj7wBicZvkG5pUY/r1965RUUNkvSUW+3JRdXiKnFwYTxpixqZiM5r1oXJR6ZhGafZUXrFFQOJGqYDY3hh5afeZcZnhdFf9Z2zJGxW8LZhF0RCXe/UTD1znQ63SIJ6IbqUfMihkYMyDrHGRqxlxG6NUy0w1W+YcikIZR/ZsLge0ybegIcp0Xh0R/7Mk/MEYd4KaQKXUnlnw/j2uQyo5hLfRULFUD82SHVuJV5JL2FUgxCF6zTSUTP1lDCinnROzYPZIZW/CDy9rKbENF1DfM63IAwE7YY/IRg==
- Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=selector1 header.d=citrix.com header.i="@citrix.com" header.h="From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck"
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
- Delivery-date: Tue, 16 Jun 2026 09:08:14 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Mon, Jun 15, 2026 at 04:15:36PM +0200, Jan Beulich wrote:
> Like is already done for I/O ports on x86 and for IRQ unbinding, check
> only the requesting domain's permissions (for it to not interfere with
> MMIO backed by another stubdom DM), but not the target domain's: Removal
> should be okay even (perhaps: especially) when permissions were already
> revoked.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -436,11 +436,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
> goto domctl_out_unlock_rcuonly;
> #endif
>
> + /*
> + * NB: The double lock isn't really needed when !add, but is used
> anyway
> + * to keep things simple.
> + */
> iocaps_double_lock(d, false);
>
> ret = -EPERM;
> if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ||
> - !iomem_access_permitted(d, mfn, mfn_end) )
> + (add && !iomem_access_permitted(d, mfn, mfn_end)) )
You seem to be doing the opposite of what the commit message states
here, and checking for permissions on the target domain, not
permissions of the requesting domain?
XEN_DOMCTL_ioport_mapping does check against current->domain, and not
against d.
FWIW, we could also remove one branch here by doing:
ret = -EPERM
if ( add && iomem_access_permitted(current->domain, mfn, mfn_end) )
{
/* add logic. */
}
else if ( !add )
{
/* remove logic. */
}
Thanks, Roger.
|
