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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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?

> + */
> +static bool __init check_xsm_signature(const void *fdt, int node,
> +                                       const char *name,
> +                                       u32 address_cells, u32 size_cells)
> +{
> +#ifdef CONFIG_FLASK
> +    u32 xen_magic = XSM_MAGIC, target_len = 8;
> +    const struct fdt_property *prop;
> +    paddr_t start, size;
> +    const __be32 *cell;
> +    char buff[16];
> +    int len;
> +
> +    prop = fdt_get_property(fdt, node, "reg", &len);
> +    if ( !prop )
> +        panic("node %s missing `reg' property\n", name);

Why panic? Can't you just return?

> +
> +    if ( len < dt_cells_to_size(address_cells + size_cells) )
> +        panic("fdt: node `%s': `reg` property length is too short\n", name);

Ditto?

> +
> +    cell = (const __be32 *)prop->data;
> +    device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> +
> +    copy_from_paddr(buff, start, sizeof(buff));
> +
> +    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;
> +
> +    return 1;
> +#else
> +    return 0;
> +#endif
> +}
> +
>  static void __init process_multiboot_node(const void *fdt, int node,
>                                            const char *name,
>                                            u32 address_cells, u32 size_cells)
> @@ -186,7 +232,13 @@ static void __init process_multiboot_node(const void 
> *fdt, int node,
>      else
>          kind = BOOTMOD_UNKNOWN;
>  
> -    /* Guess that first two unknown are kernel and ramdisk respectively. */
> +    /**
> +     * 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 have not initrd.
> +     */
>      if ( kind == BOOTMOD_UNKNOWN )
>      {
>          switch ( kind_guess++ )
> @@ -195,6 +247,9 @@ static void __init process_multiboot_node(const void 
> *fdt, int node,
>          case 1: kind = BOOTMOD_RAMDISK; break;
>          default: break;
>          }
> +        if (kind_guess > 1 && check_xsm_signature(fdt, node, name,
> +                                                  address_cells, size_cells))
> +            kind = BOOTMOD_XSM;
>      }
>  
>      prop = fdt_get_property(fdt, node, "reg", &len);
> -- 
> 2.5.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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