Re: [Xen-devel] [PATCH v2] xen/arm64: check XSM Magic and Signature from the second unknown module.

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 03/28/2016 04:54 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 29, 2016 at 01:55:07AM +0800, fu.wei@xxxxxxxxxx wrote:
> > From: Fu Wei <fu.wei@xxxxxxxxxx>
> >
> > This patch add a check_xsm_signature static function for detecting XSM
> > from the second unknown module.
> >
> > If xen can't get the kind of module from compatible, we guess the kind of
> > these first two unknown respectively:
> >      (1) The first unknown must be kernel;
> >      (2) The second unknown is ramdisk, only if we have ramdisk;
> >      (3) Start from the 2nd unknown, detect the XSM binary signature;
> >      (4) If we got XSM in the 2nd unknown, that means we don't load initrd.
> Pls make the 'xen' be 'Xen'.
>
> > Signed-off-by: Fu Wei <fu.wei@xxxxxxxxxx>
> Cc-ing also Daniel (XSM maintainer).
>
> And Julien (linaro.org != arm.com) and Stefano.
> > ---
> > v2: Using XEN_MAGIC macro instead of 0xf97cff8c :
> >      uint32_t selinux_magic = 0xf97cff8c; --> uint32_t xen_magic = XEN_MAGIC;
> >      Comment out the code(return 0 directly), if CONFIG_FLASK is not set.
> >
> > v1: http://lists.xen.org/archives/html/xen-devel/2016-03/msg02430.html
> >      The first upstream patch to xen-devel mailing lists.
> >
> >   xen/arch/arm/bootfdt.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> > index 8a14015..322f17f 100644
> > --- a/xen/arch/arm/bootfdt.c
> > +++ b/xen/arch/arm/bootfdt.c
> > @@ -163,6 +163,52 @@ static void __init process_memory_node(const void *fdt, 
> > int node,
> >       }
> >   }
> >
> > +/**
> > + * check_xsm_signature - Check XSM Magic and Signature of the module header
> > + * A XSM module has a special header
> > + * ------------------------------------------------
> > + * uint magic | uint target_len | uchar target[8] |
> > + * 0xf97cff8c |        8        |    "XenFlask"   |
> > + * ------------------------------------------------
> > + * 0xf97cff8c is policy magic number.
> > + * So we only read the first 16 Bytes of the module, then check these three
> s/Bytes/bytes/
> > + * parts.
> Is it possible for the hypervisor to chnage the policy magic number? Perhaps
> you should have :
>
> BUILD_BUG_ON(0xf97cff8c != XSM_MAGIC);
>
> to guard against changes?
The value of XSM_MAGIC will always be that constant if FLASK is the enabled
security module; the value was different when the (now-removed) ACM module
was selected.

[...]
> > +    if (strncmp(buff, (char *) &xen_magic, sizeof(u32)) ||
> > +        strncmp(buff + sizeof(u32), (char *) &target_len, sizeof(u32)) ||
> > +        strncmp(buff + sizeof(u32) * 2, "XenFlask", target_len))
> > +        return 0;
> > +
memcmp() is more correct than strncmp() here, especially since target_len will
have embedded NULLs.  It also assumes little endian byte order; is that worth
commenting on?

--
Daniel De Graaf
National Security Agency

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