Re: [PATCH v4 2/3] xsm: consolidate loading the policy buffer

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.06.2022 15:47, Daniel P. Smith wrote:
> 
> On 6/2/22 05:47, Jan Beulich wrote:
>> On 31.05.2022 20:20, Daniel P. Smith wrote:
>>> Previously, initializing the policy buffer was split between two functions,
>>> xsm_{multiboot,dt}_policy_init() and xsm_core_init(). The latter for loading
>>> the policy from boot modules and the former for falling back to built-in
>>> policy.
>>>
>>> This patch moves all policy buffer initialization logic under the
>>> xsm_{multiboot,dt}_policy_init() functions. It then ensures that an error
>>> message is printed for every error condition that may occur in the 
>>> functions.
>>> With all policy buffer init contained and only called when the policy buffer
>>> must be populated, the respective xsm_{mb,dt}_init() functions will panic 
>>> for
>>> all errors except ENOENT. An ENOENT signifies that a policy file could not 
>>> be
>>> located. Since it is not possible to know if late loading of the policy 
>>> file is
>>> intended, a warning is reported and XSM initialization is continued.
>>
>> Is it really not possible to know? flask_init() panics in the one case
>> where a policy is strictly required. And I'm not convinced it is
>> appropriate to issue both an error and a warning in most (all?) of the
>> other cases (and it should be at most one of the two anyway imo).
> 
> With how XSM currently works, I do not see how without creating a
> layering violation, as you had mentioned before. It is possible for
> flask_init() to know because the flask= parameter belongs to the flask
> policy module and can be directly checked.
> 
> I think we view this differently. A call to
> xsm_{multiboot,dt}_policy_init() is asking for a policy file to be
> loaded. If it is not able to do so is an error. This error is reported
> back to xsm_{multiboot,dt}_init() which is responsible for initializing
> the XSM framework. If it encounters an error for which inhibits it from
> initializing XSM would be an error whereas an error it encounters that
> does not block initialization should warn the user as such. While it is
> true that the only error for the xsm_multiboot_policy_init() currently
> is a failure to locate a policy file, ENOENT, I don't see how that
> changes the understanding.

Well, I think that to avoid the layering violation the decision whether
an error is severe enough to warrant a warning (or is even fatal) needs
to be left to the specific model (i.e. Flask in this case).

Jan