Re: [Xen-devel] [PATCH] xsm: hide detailed Xen version from unprivileged guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.01.2020 12:02, Sergey Dyasli wrote:
> On 06/01/2020 14:40, Jan Beulich wrote:
>> On 06.01.2020 15:35, Sergey Dyasli wrote:
>>> On 06/01/2020 11:28, George Dunlap wrote:
>>>> On 12/19/19 11:15 PM, Andrew Cooper wrote:
>>>>> On 19/12/2019 11:35, Jan Beulich wrote:
>>>>>>>>>     XENVER_changeset
>>>>>>>>>     XENVER_commandline
>>>>>>>>>     XENVER_build_id
>>>>>>>>>
>>>>>>>>> Return a more customer friendly empty string instead of "<denied>"
>>>>>>>>> which would be shown in tools like dmidecode.>
>>>>>>>> I think "<denied>" is quite fine for many of the original purposes.
>>>>>>>> Maybe it would be better to filter for this when populating guest
>>>>>>>> DMI tables?
>>>>>>> I don't know how DMI tables are populated, but nothing stops a guest
>>>>>>> from using these hypercalls directly.
>>>>>> And this is precisely the case where I think "<denied>" is better
>>>>>> than an empty string.
>>>>>
>>>>> "<denied>" was a terrible choice back when it was introduced, and its
>>>>> still a terrible choice today.
>>>>>
>>>>> These are ASCII string fields, and the empty string is a perfectly good
>>>>> string.  Nothing is going to break, because it would have broken the
>>>>> first time around.
>>>>>
>>>>> The end result without denied sprayed all over this interface is much
>>>>> cleaner overall.
>>>>
>>>> Unfortunately this mail doesn't contain any facts or arguments, just
>>>> unsubstantiated value judgements.  What's so terrible about "<denied>"
>>>> -- what bad effect does it have?  Why is "" better / cleaner?
>>>
>>> It can be explained with a picture (attached) ;)
>>
>> But that's something better addressed at or close to the presentation
>> layer, not deep down in Xen.
> 
> I agree with that. And looks like the following diff does the trick:
> 
> diff --git a/tools/firmware/hvmloader/smbios.c 
> b/tools/firmware/hvmloader/smbios.c
> index 97a054e9e3..b4d72c375f 100644
> --- a/tools/firmware/hvmloader/smbios.c
> +++ b/tools/firmware/hvmloader/smbios.c
> @@ -275,6 +275,8 @@ hvm_write_smbios_tables(
>      xen_minor_version = (uint16_t) xen_version;
> 
>      hypercall_xen_version(XENVER_extraversion, xen_extra_version);
> +    if ( strcmp(xen_extra_version, "<denied>") == 0 )
> +        memset(xen_extra_version, 0, sizeof(xen_extra_version));
> 
>      /* build up human-readable Xen version string */
>      p = xen_version_str;

When you submit this as a proper patch, feel free to add my ack
right away (as long you give it a non-empty and half way useful
description).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel