Re: [Xen-devel] [PATCH 2/2] xen/livepatch: Don't crash on encountering STN_UNDEF relocations

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/06/17 11:11, Jan Beulich wrote:
>>>> On 13.06.17 at 22:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/livepatch.c
>> +++ b/xen/arch/x86/livepatch.c
>> @@ -170,14 +170,22 @@ int arch_livepatch_perform_rela(struct livepatch_elf 
>> *elf,
>>          uint8_t *dest = base->load_addr + r->r_offset;
>>          uint64_t val;
>>  
>> -        if ( symndx > elf->nsym )
>> +        if ( symndx == STN_UNDEF )
>> +            val = 0;
>> +        else if ( symndx > elf->nsym )
>>          {
>>              dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants 
>> symbol@%u which is past end!\n",
>>                      elf->name, symndx);
>>              return -EINVAL;
>>          }
>> -
>> -        val = r->r_addend + elf->sym[symndx].sym->st_value;
>> +        else if ( !elf->sym[symndx].sym )
>> +        {
>> +            dprintk(XENLOG_ERR, LIVEPATCH "%s: No symbol@%u\n",
>> +                    elf->name, symndx);
>> +            return -EINVAL;
>> +        }
>> +        else
>> +            val = r->r_addend + elf->sym[symndx].sym->st_value;
> I don't understand this: st_value for STN_UNDEF is going to be zero
> (so far there's also no extension defined for the first entry, afaict),
> so there should be no difference between hard-coding the zero and
> reading the symbol table entry. Furthermore r_addend would still
> need applying. And finally "val" is never being cast to a pointer, and
> hence I miss the connection to whatever crash you've been
> observing.

elf->sym[0].sym is the NULL pointer.

->st_value dereferences it.

~Andrew

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