Re: [Xen-devel] [PATCH 15/15] xsm: add a default policy to .init.data

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.06.16 at 18:58, <dgdegra@xxxxxxxxxxxxx> wrote:
> On 06/09/2016 11:30 AM, Andrew Cooper wrote:
>> On 09/06/16 15:47, Daniel De Graaf wrote:
>>> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
>>> index 4a264c2..6ffccb2 100644
>>> --- a/xen/xsm/xsm_core.c
>>> +++ b/xen/xsm/xsm_core.c
>>> @@ -36,6 +36,17 @@ static inline int verify(struct xsm_operations *ops)
>>>      return 0;
>>>  }
>>>
>>> +extern char __xsm_init_policy_start[], __xsm_init_policy_end[];
>>> +
>>> +static void __init xsm_policy_init(void)
>>> +{
>>> +    if ( policy_size == 0 && __xsm_init_policy_end != 
>>> __xsm_init_policy_start )
>>> +    {
>>> +        policy_buffer = __xsm_init_policy_start;
>>
>> I think this might cause a problem for ARM.
>>
>> xsm_dt_init(void) does
>>
>> ret = xsm_core_init();
>> xfree(policy_buffer);
>>
>> which might now try freeing a piece of initdata.  Even going back to c/s
>> a8f2fb51c19 which introduced this code, I can't spot its justification.
> 
> The buffer is allocated and populated in xsm_dt_policy_init.

Only if there is an XSM module; I think Andrew refers to the
assignment to policy_buffer still visible above, which would
remain untouched by xsm_dt_policy_init() if bailing early.

>> It also looks like the entire policy buffer can be const, at which point
>> it can be linked slightly higher in .init.data with the other
>> logically-const data.
> 
> Sure, although this will require quite a bit of const propagation down into
> the FLASK security server (or casting it away).

No casting away of constness please, except in very special situations
(which this one pretty clearly isn't).

Jan


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