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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.06.17 at 20:13, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/livepatch.c
> +++ b/xen/arch/x86/livepatch.c
> @@ -170,14 +170,27 @@ int arch_livepatch_perform_rela(struct livepatch_elf 
> *elf,
>          uint8_t *dest = base->load_addr + r->r_offset;
>          uint64_t val;
>  
> +        if ( symndx == STN_UNDEF )
> +        {
> +            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
> +                    elf->name);
> +            return -EOPNOTSUPP;
> +        }
> +
>          if ( symndx > elf->nsym )

Would you mind fixing the off-by-one mistake here at once?

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

With this it may also be a good idea to have elf_get_sym() set
sym[0].sym (and sym[0].name) to NULL.

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

In the spirit of the earlier code here I'd suggest omitting both "else".

Jan


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