Re: [Xen-devel] [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.09.16 at 16:13, <julien.grall@xxxxxxx> wrote:

> 
> On 19/09/2016 16:11, Jan Beulich wrote:
>>>>> On 19.09.16 at 15:33, <julien.grall@xxxxxxx> wrote:
>>> On 19/09/2016 11:27, Jan Beulich wrote:
>>>>>>> On 16.09.16 at 18:38, <konrad.wilk@xxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/arm/livepatch.c
>>>>> +++ b/xen/arch/arm/livepatch.c
>>>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct 
>>>>> livepatch_elf
>>> *elf,
>>>>>      return true;
>>>>>  }
>>>>>
>>>>> +bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf,
>>>>> +                                const struct livepatch_elf_sym *sym)
>>>>> +{
>>>>> +#ifdef CONFIG_ARM_32
>>>>> +    /*
>>>>> +     * Xen does not use Thumb instructions - and we should not see any of
>>>>> +     * them. If we do, abort.
>>>>> +     */
>>>>> +    if ( sym->name && *sym->name == '$' && sym->name[1] == 't' )
>>>
>>> Please use sym->name[0] for readability. Also, you may want to check the
>>> length of the symbol before checking the second character.
>>
>> Why would the length check be needed? If the first character is $,
>> then looking at the second one is always valid (and it being nul will
>> be properly dealt with by the expression above).
> 
> Because you may have a payload which is not valid? Or maybe you consider 
> that all the payload are trusted.

If all symbols' names are inside their string tables and the string
tables are both contained inside the image and have a zero byte
at their end (all of which gets verified afair), nothing bad can
happen I think.

Jan


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