Re: [PATCH 3/6] xsm: enabling xsm to always be included

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>,
	"Daniel P. Smith" <dpsmith@apertussolutions.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>, "Wei Liu" <wl@xen.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Tamas K Lengyel" <tamas@tklengyel.com>,
	"Tim Deegan" <tim@xen.org>, "Juergen Gross" <jgross@suse.com>,
	"Alexandru Isaila" <aisaila@bitdefender.com>,
	"Petre Pircalabu" <ppircalabu@bitdefender.com>,
	"Dario Faggioli" <dfaggioli@suse.com>,
	"Paul Durrant" <paul@xen.org>,
	"Daniel De Graaf" <dgdegra@tycho.nsa.gov>,
	persaur@gmail.com, christopher.w.clark@gmail.com,
	adam.schwalm@starlab.io, scott.davis@starlab.io,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 3/6] xsm: enabling xsm to always be included
Date: Mon, 21 Jun 2021 11:41:21 +0100	[thread overview]
Message-ID: <8a909d6b-e69c-05ce-35dd-0f6be719b5ae@citrix.com> (raw)
In-Reply-To: <6d50efc1-6c13-1481-b70c-0abfa99aa610@suse.com>

On 21/06/2021 07:58, Jan Beulich wrote:
> On 18.06.2021 22:27, Daniel P. Smith wrote:
>> On 6/18/21 8:26 AM, Jan Beulich wrote:
>>> On 18.06.2021 01:39, Daniel P. Smith wrote:
>>>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>>>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>>>> pointers to static functions. As such this commit,
>>>>  * eliminates CONFIG_XSM
>>> Following from what Andrew has said (including him mentioning your
>>> changing of certain Kconfig option defaults), I'm not convinced this is
>>> a good move. This still ought to serve as the overall XSM-yes-or-no
>>> setting. If internally you make said two variants match in behavior,
>>> that's a different thing.
>> Apologies that I did not express this clearly. What I was attempting to
>> say is the fact of the matter is that there is no logical behavior
>> difference between "XSM no" and "XSM yes with dummy policy". The only
>> difference is the mechanics of how the dummy functions get called.
>> Specifically via macro magic the dummy functions are either flipped into
>> static inline declarations that are then included into the code where
>> they are invoked or the macro magic has them ending up in the dummy.c
>> XSM module where they are wrapped in macro generated functions that are
>> set as the functions in the dummy xsm_ops structure. Thus it is always
>> the same logic being invoked, it is just mechanics of how you get to the
>> logic.
> That's what I understood, really. What I dislike is the inline functions
> going away in what we currently call !XSM.

I'm sorry, but this is an unreasonable objection.

The mess used to create the status quo *is* the majority reason why
fixing/developing XSM is so hard, and why the code is so obfuscated.  To
prove this point, how many people on this email thread realise that
calls using XSM_HOOK offer 0 security under xsm_default_action()?

Having xsm_default_action() forced inline isn't obviously the right move
in the first place, and I doubt that you could even measure a
performance difference for using real function calls.

Even if there is a marginal performance difference, and I doubt that
there is, performance is far less important than de-obfuscating the code
and fixing our various security mechanisms to be first-class supported
citizens.

~Andrew