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

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

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;

--
Thanks,
Sergey

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