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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.06.2021 12:41, Andrew Cooper wrote:
> 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.

What I don't understand from all you say is why you think that having
an as-if-no-XSM build configuration, without any way to switch to an
alternative model (i.e. the XSM=n that we have right now), is a bad
thing. I don't mind the XSM=y case getting improved, but I don't see
(yet) why it is a good thing to force this onto everyone.

Jan