All of lore.kernel.org
 help / color / mirror / Atom feed
From: George Dunlap <George.Dunlap@citrix.com>
To: Doug Goldstein <cardoe@cardoe.com>
Cc: Lars Kurth <lars.kurth@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Sergey Dyasli <sergey.dyasli@citrix.com>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>,
	Jan Beulich <jbeulich@suse.com>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [Xen-devel] [PATCH v2] xsm: hide detailed Xen version from unprivileged guests
Date: Sat, 11 Jan 2020 09:35:18 +0000	[thread overview]
Message-ID: <F99E90AE-2468-4401-A0DD-AB50D84C2A3B@citrix.com> (raw)
In-Reply-To: <cbe5fc46-4fc9-f905-3492-2abc34a8e12e@cardoe.com>



> On Jan 11, 2020, at 3:55 AM, Doug Goldstein <cardoe@cardoe.com> 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@citrix.com>
>>>> ---
>>>> 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@citrix.com>
>>>> CC: George Dunlap <George.Dunlap@eu.citrix.com>
>>>> CC: Ian Jackson <ian.jackson@eu.citrix.com>
>>>> CC: Jan Beulich <jbeulich@suse.com>
>>>> CC: Julien Grall <julien@xen.org>
>>>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>>> CC: Wei Liu <wl@xen.org>
>>>> CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> 
>>> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2020-01-11  9:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10 10:37 [Xen-devel] [PATCH v2] xsm: hide detailed Xen version from unprivileged guests Sergey Dyasli
2020-01-10 11:02 ` Andrew Cooper
2020-01-10 15:28   ` George Dunlap
2020-01-10 15:56     ` Jan Beulich
2020-01-10 16:45       ` Jürgen Groß
2020-01-10 17:00         ` George Dunlap
2020-01-11  3:55     ` Doug Goldstein
2020-01-11  9:35       ` George Dunlap [this message]
2020-01-13 11:01   ` Sergey Dyasli
2020-01-10 11:09 ` Jan Beulich
2020-01-11  4:02 ` Doug Goldstein
2020-01-11  9:02   ` George Dunlap
2020-01-12 18:26     ` Doug Goldstein
2020-01-13 12:51       ` George Dunlap
2020-01-13 13:39         ` Julien Grall
2020-01-13 14:01           ` Andrew Cooper
2020-01-13 14:07             ` George Dunlap
2020-01-13 14:28               ` Julien Grall
2020-01-13 14:40         ` Andrew Cooper
2020-01-14 10:19           ` Sergey Dyasli
2020-01-13 14:52         ` Julien Grall
2020-01-13 14:01       ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F99E90AE-2468-4401-A0DD-AB50D84C2A3B@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=Andrew.Cooper3@citrix.com \
    --cc=Ian.Jackson@citrix.com \
    --cc=cardoe@cardoe.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=konrad.wilk@oracle.com \
    --cc=lars.kurth@citrix.com \
    --cc=sergey.dyasli@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.