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

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> CCing Daniel De Graaf, as the maintainer of this code.
>
> However FWIW,
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> > ---
> >   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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel