Re: [PATCH] xen/xsm: Generate the permission in a spec-compliant way

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 02/23/2015 10:04 AM, Julien Grall wrote:
> Hi Daniel,
>
> On 20/02/15 23:01, Daniel De Graaf wrote:
>> On 02/20/2015 10:58 AM, Julien Grall wrote:
>>> Each class can contains 32 permisions which are encoded on a word (one
>>> bit per permission).
>>>
>>> Currently the awk script will generate an hexadecimal value for each
>>> permission. This may result to generate an invalid value on some version
>>> of awk.
>>>
>>> For instance debian jessie is using a version of mawk where (1 << 31)
>>> will result to 0x7fffffff.
>>>
>>> This is because the awk specification requires to do the arithmetic with
>>> float. So the resulting integer may vary following the implementation.
>>>
>>> As the generated headers are only used by C code, generate the
>>> permission define via "1UL << n".
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> The fix looks correct.  For backporting: this is only a problem since the
>> auto-generation was moved into the hypervisor build (between 4.2 and 4.3).
>> Prior to this, the headers were manually generated, and apparently nobody
>> ran the script on a system with this bug - in part because nobody ran
... because nobody ran it.

>>
>> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>
>> Wow, that's quite an annoying bug.  Thankfully, it's more likely to make a
>> broken system than an insecure one, since doing an access check on the
>> permission 0x7fffffff will result in checking for access to all 31 other
>> permissions instead of the one you intended to check for.  For Xen, it
>> looks like this is unlikely to succeed, and also won't do something like
>> prevent the system from booting:
>
> Actually I think the policy is not even loaded.
>
>>From the log I got
> (XEN) Flask:  Initializing.
> (XEN) AVC INITIALIZED
> (XEN) Flask: 128 avtab hash slots, 278 rules.
> (XEN) Flask: 128 avtab hash slots, 278 rules.
> (XEN) Flask:  3 users, 3 roles, 39 types, 1 bools
> (XEN) Flask:  12 classes, 278 rules
> (XEN) Flask:  permission setscheduler in class xen has incorrect value
> (XEN) Flask:  the definition of a class is incorrect
> (XEN) Flask:  Starting in enforcing mode.
>
> As the policy is not valid (see validate_classes in
> security_load_policy), we bail out directly.
>
> But I don't understand why we continue to boot and everything is
> working. Flask is not even correctly initialized...
>
> Did I miss something?

It seems that failure returns from the XSM initcall are never checked or
acted upon; xsm_core.c just runs "(*call) ();" and ignores the return.
This should probably be fixed; I'm going to send a patch to address this
by changing the return value to void and calling panic if an invalid
policy is specified at boot.

-- 
Daniel De Graaf
National Security Agency