linux-kernel - Re: [PATCH] drivers/xen/hypervisor: Expose VM SIF flags to userspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <358e49fa-8ce7-67ce-8e0b-e523dee9ea19@suse.com>
Date:   Tue, 29 Nov 2022 16:27:28 +0100
From:   Juergen Gross <jgross@...e.com>
To:     Per Bilse <per.bilse@...rix.com>, linux-kernel@...r.kernel.org
Cc:     Stefano Stabellini <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Jan Beulich <jbeulich@...e.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH] drivers/xen/hypervisor: Expose VM SIF flags to userspace

On 29.11.22 16:00, Per Bilse wrote:
> /proc/xen is a legacy pseudo filesystem which predates Xen support
> getting merged into Linux.  It has largely been replaced with more
> normal locations for data (/sys/hypervisor/ for info, /dev/xen/ for
> user devices).  We want to compile xenfs support out of the dom0 kernel.
> 
> There is one item which only exists in /proc/xen, namely
> /proc/xen/capabilities with "control_d" being the signal of "you're in
> the control domain".  This ultimately comes from the SIF flags provided
> at VM start.
> 
> This patch exposes all SIF flags in /sys/hypervisor/properties/flags,
> which will coexist with /proc/xen while dependencies are being migrated.
> Possible values are "privileged", "initdomain", "multiboot",
> "mod_start_pfn", and "virtmap", with "initdomain" being the equivalent
> of "control_d".
> 
> Signed-off-by: Per Bilse <per.bilse@...rix.com>
> ---
>   drivers/xen/sys-hypervisor.c | 26 ++++++++++++++++++++++++++
>   include/xen/interface/xen.h  | 13 ++++++++-----
>   2 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> index fcb0792f090e..7393e04bdb6d 100644
> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -379,6 +379,31 @@ static ssize_t buildid_show(struct hyp_sysfs_attr *attr, char *buffer)
>   
>   HYPERVISOR_ATTR_RO(buildid);
>   
> +static ssize_t flags_show(struct hyp_sysfs_attr *attr, char *buffer)
> +{
> +	static char const *const sifstr[SIFN_NUM_SIFN] = {
> +		[SIFN_PRIV]  = "privileged",
> +		[SIFN_INIT]  = "initdomain",
> +		[SIFN_MULTI] = "multiboot",
> +		[SIFN_PFN]   = "mod_start_pfn",
> +		[SIFN_VIRT]  = "virtmap"
> +	};
> +	unsigned sifnum, sifmask;
> +	ssize_t ret = 0;
> +
> +	sifmask = ~(~0U << SIFN_NUM_SIFN);  // ...0000111...
> +	if (xen_domain() && (xen_start_flags & sifmask) != 0) {
> +		for (sifnum = 0; sifnum != SIFN_NUM_SIFN; sifnum++) {
> +			if ((xen_start_flags & (1<<sifnum)) != 0)
> +				ret += sprintf(buffer+ret, "%s ", sifstr[sifnum]);
> +		}
> +		buffer[ret-1] = '\n';
> +	}
> +	return ret;
> +}
> +
> +HYPERVISOR_ATTR_RO(flags);
> +
>   static struct attribute *xen_properties_attrs[] = {
>   	&capabilities_attr.attr,
>   	&changeset_attr.attr,
> @@ -386,6 +411,7 @@ static struct attribute *xen_properties_attrs[] = {
>   	&pagesize_attr.attr,
>   	&features_attr.attr,
>   	&buildid_attr.attr,
> +	&flags_attr.attr,
>   	NULL
>   };
>   
> diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
> index 0ca23eca2a9c..762a348abe3e 100644
> --- a/include/xen/interface/xen.h
> +++ b/include/xen/interface/xen.h
> @@ -648,11 +648,14 @@ struct start_info {
>   };
>   
>   /* These flags are passed in the 'flags' field of start_info_t. */
> -#define SIF_PRIVILEGED      (1<<0)  /* Is the domain privileged? */
> -#define SIF_INITDOMAIN      (1<<1)  /* Is this the initial control domain? */
> -#define SIF_MULTIBOOT_MOD   (1<<2)  /* Is mod_start a multiboot module? */
> -#define SIF_MOD_START_PFN   (1<<3)  /* Is mod_start a PFN? */
> -#define SIF_VIRT_P2M_4TOOLS (1<<4)  /* Do Xen tools understand a virt. mapped */
> +/* Text strings are printed out in sys-hypervisor.c, we guard   */
> +/* against mix-ups and errors by enumerating the flags.         */
> +enum { SIFN_PRIV, SIFN_INIT, SIFN_MULTI, SIFN_PFN, SIFN_VIRT, SIFN_NUM_SIFN };
> +#define SIF_PRIVILEGED      (1<<SIFN_PRIV)  /* Is the domain privileged? */
> +#define SIF_INITDOMAIN      (1<<SIFN_INIT)  /* Is this the initial control domain? */
> +#define SIF_MULTIBOOT_MOD   (1<<SIFN_MULTI) /* Is mod_start a multiboot module? */
> +#define SIF_MOD_START_PFN   (1<<SIFN_PFN)   /* Is mod_start a PFN? */
> +#define SIF_VIRT_P2M_4TOOLS (1<<SIFN_VIRT)  /* Do Xen tools understand a virt. mapped */

Please don't change this header, as it is based on its master
located in the Xen repository.

An acceptable solution would be to send a Xen patch first doing the
xen.h changes, and when that patch has been taken to modify the
related Linux header accordingly.

In case you want to go that route, please add a "XEN_" prefix to the
enum members.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)