linux-kernel - Re: [PATCH] drivers/xen/hypervisor: Expose Xen 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] [day] [month] [year] [list]

Date:   Mon, 12 Dec 2022 13:35:22 +0000
From:   Per Bilse <Per.Bilse@...rix.com>
To:     Juergen Gross <jgross@...e.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     Stefano Stabellini <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH] drivers/xen/hypervisor: Expose Xen SIF flags to userspace

On 09/12/2022 09:04, Juergen Gross wrote:
> On 02.12.22 19:22, Per Bilse wrote:
>> +Description:    If running under Xen:
>> +        All bits in Xen's start-flags are represented as
>> +        boolean files, returning '1' if set, '0' otherwise.
> 
> I think at least the files which want to be used by e.g. systemd
> ("initdomain" as replacement for the "control_d" string in capabilities,
> but I think "privileged" as well) should be explicitly added to this
> description, as those are meant to be used as a stable ABI.

OK, yes, it's a bit vague as is.

>> +static struct hyp_sysfs_attr flag_attrs[FLAG_COUNT] = {
>> +    [ilog2(SIF_PRIVILEGED)] = {
>> +        .attr = { .name = "privileged", .mode = 0444 },
>> +        .show = flag_show,
>> +        .hyp_attr_value = SIF_PRIVILEGED
>> +    },
> 
> What about:
> 
> #define FLAG_NODE(bit, node)                    \
>      [ilog2(bit)] = {                    \
>          .attr = { .name = #node, .mode = 0444 },    \
>          .show = flag_show,                \
>          .hyp_attr_value = bit                \
>      }

Yes ... I was thinking that maybe flags wouldn't normally share most 
characteristics, but they probably do.

> In order to avoid a consumer having to look into the sources for any other
> set flag, maybe add names for currently defined flags, too? Or just skip > the other flags and add a single additional node ("flags"?) with the 
whole
> value of xen_start_flags either in hex or binary form?

The current format is as suggested by Andrew Cooper in email about a 
previous incarnation 
(https://lore.kernel.org/lkml/92300a81-b97b-f5d6-e3e8-363d8a7d3742@citrix.com/); 
specifically, that of the known flags only initdomain and privileged are 
relevant outside a kernel context, and that all other flags be exposed 
individually.

> Please note that this is a suggestion only, I'm not insisting on it.

Please, many thanks for the feedback, it's always good to hear other 
people's thoughts.

Best,

   -- Per