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

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> > 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@xxxxxxxxxxxxx>
> >
> > 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

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