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

[Fu Wei <fu.wei@xxxxxxxxxx>]

Hi Konrad,

On 29 March 2016 at 04:54, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 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'.

Thanks,  :-)

>
>> Signed-off-by: Fu Wei <fu.wei@xxxxxxxxxx>
>
> Cc-ing also Daniel (XSM maintainer).

Thanks, I will add him into to-list next time:-)


>
> 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/

thanks , will do

>> + * 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?

Good point, will do

Thanks for your review :-)

>
>> +
>> +    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



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

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