Re: [PATCH] Fix boot crash on xsm/flask enabled builds when no policy module is present

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

On 08/26/2013 06:52 AM, Andrew Cooper wrote:
> On 26/08/2013 11:03, Tomasz Wroblewski wrote:
>> Xen crashes on boot of xsm/flask enabled builds, if policy module is not specified.
>> This seems to have worked on 4.1 at least. Can be fixed by testing whether policy_buffer
>> is NULL before attempting to load from it - it's a global which is set to non-NULL when
>> policy module is detected.
>>
>> Signed-off-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>
>
> CCing Daniel De Graaf, as the maintainer of this code.
>
> However FWIW,
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
>> ---
>>   xen/xsm/flask/hooks.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
>> index fa0589a..cfa2929 100644
>> --- a/xen/xsm/flask/hooks.c
>> +++ b/xen/xsm/flask/hooks.c
>> @@ -1585,7 +1585,8 @@ static __init int flask_init(void)
>>       if ( register_xsm(&flask_ops) )
>>           panic("Flask: Unable to register with XSM.\n");
>>
>> -    ret = security_load_policy(policy_buffer, policy_size);
>> +    if ( policy_buffer )
>> +        ret = security_load_policy(policy_buffer, policy_size);
>>
>>       if ( flask_enforcing )
>>           printk("Flask:  Starting in enforcing mode.\n");
>

While this change is not wrong, I also don't see how it could fix
anything. The security_load_policy function will not dereference
policy_buffer if policy_size is zero (it'll return -EINVAL first),
and the only location that sets policy_size (xsm_policy_init) also
sets policy_buffer. If this function is setting the NULL pointer,
it also does a dereference - and it will be visible in the printk.

Also, on 08/26/2013 07:12 AM, Jan Beulich wrote:
> Question is whether policy_buffer == NULL really isn't supposed
> to result in a -E... return value (as in fact flask initialization failed).

The return value of flask_init isn't ever checked. A failure to
load the policy at boot is identical to no policy - waiting for a
flask_disable or "xl loadpolicy" hypercall from dom0.

-- 
Daniel De Graaf
National Security Agency