[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




> On Jan 11, 2020, at 3:55 AM, Doug Goldstein <cardoe@xxxxxxxxxx> wrote:
> 
> 
> 
> On 1/10/20 9:28 AM, George Dunlap wrote:
>> On 1/10/20 11:02 AM, Andrew Cooper wrote:
>>> On 10/01/2020 10:37, Sergey Dyasli wrote:
>>>> Hide the following information that can help identify the running Xen
>>>> binary version: XENVER_extraversion, XENVER_compile_info, XENVER_changeset.
>>>> Add explicit cases for XENVER_commandline and XENVER_build_id as well.
>>>> 
>>>> Introduce xsm_filter_denied() to hvmloader to remove "<denied>" string
>>>> from guest's DMI tables that otherwise would be shown in tools like
>>>> dmidecode.
>>>> 
>>>> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
>>>> ---
>>>> v1 --> v2:
>>>> - Added xsm_filter_denied() to hvmloader instead of modifying xen_deny()
>>>> - Made behaviour the same for both Release and Debug builds
>>>> - XENVER_capabilities is no longer hided
>>>> 
>>>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
>>>> CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>>>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>>>> CC: Julien Grall <julien@xxxxxxx>
>>>> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>>> CC: Wei Liu <wl@xxxxxxx>
>>>> CC: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
>>> 
>>> I realise there are arguments over how to fix this, but we (the Xen
>>> community) have already f*cked up once here, and this is doing so a
>>> second time.
>>> 
>>> Nack.
>>> 
>>> Fixing it anywhere other than Xen is simply not appropriate.
>>> 
>>> The reason for this (which ought to be obvious, but I guess only to
>>> those who actually do customer support) is basic human physiology.
>>> "denied" means something has gone wrong.  It scares people, and causes
>>> them to seek help to change fix whatever is broken.
>> This seems like a reasonable argument that "<denied>" causes issues.
>> But that doesn't change the fact that "" also causes issues.
> 
> I'd be curious to hear the case where the empty string causes issues.

This was mentioned in the previous version of this thread.  To mirror Andy’s 
(rather inflammatory) formulation:

The reason for this (which ought to be obvious, but I guess only to those who 
actually [use computer interfaces]) is basic [user interface design].  When you 
ask an interface to do something for you, it should either do the thing you 
asked it to do, or it should tell you why it couldn’t.  Or, at *very* least, it 
should tell you *that* it didn’t.

Imagine someone who writes code to get the buildid and store it in a log 
somewhere.  Everything compiles and runs great.  And then a little ways down 
the road, they look in their log, and find “Xen 4.13.1”, with no buildid!  So 
they spend an afternoon trying to figure out what went wrong.  Did they screw 
up the build of Xen?  No, `xl info` reports the bulidid.  Did they grab they 
compile against the wrong header?  Did they screw up the call somehow?  Did 
they accidentally read it into the wrong buffer?  Imagine their rage when 
eventually, after hours of tracing through code they’ve seen a dozen times, 
they figure out that THE INTERFACE IS SILENTLY FAILING.

Or imagine someone who sells something that runs inside of Xen guests and uses 
Xen-specific features.  All of her test systems have versions of Xen which 
expose the buildid to guests.  At some point one of her customers has an issue, 
so she asks them to get the buildid by clicking here here and here, and getting 
the string.  But when the customer reports the string, there’s no buildid, 
nothing!  So again she spends hours trying to “debug” the “why do I not have a 
buildid” issue, looking like an idiot in front of her customer, only to 
discover that their customer is running a version of Xen where the buildid is 
simply empty, with no indication that anything has been hidden.

Silently returning a result which is indistinguishable from “no buildid exists” 
is just plain rude to any programmer who uses this interface.

I completely understand that having “<denied>” show up in a GUI might cause 
customer support headaches.  But fundamentally it’s not the hypervisor’s job to 
coddle timid end-users; that’s what UIs are for.

I don’t want to be dogmatic about that principle, so I’m happy to entertain 
changing the string to be something more user-friendly (or some other suitable 
change in the interface).  But I do want to be dogmatic about interfaces not 
silently failing.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.