>>> On 22.06.17 at 20:15, <andrew.coop...@citrix.com> wrote:
> A symndx of STN_UNDEF is special, and means a symbol value of 0. While
> legitimate in the ELF standard, its existance in a livepatch is questionable
> at best. Until a plausible usecase presents itself, reject such a relocation
> with -EOPNOTSUPP.
>
> Additionally, fix an off-by-one error while range checking symndx, and perform
> a safety check on elf->sym[symndx].sym before derefencing it, to avoid
> tripping over a NULL pointer when calculating val.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
with two remarks:
> --- a/xen/arch/x86/livepatch.c
> +++ b/xen/arch/x86/livepatch.c
> @@ -170,12 +170,24 @@ 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 )
> + {
> + dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
> + elf->name);
> + return -EOPNOTSUPP;
> + }
> + 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;
> }
> + else if ( !elf->sym[symndx].sym )
Neither of the two "else" is really necessary, and elsewhere we've
been telling people to avoid such.
> + {
> + dprintk(XENLOG_ERR, LIVEPATCH "%s: No symbol@%u\n",
Symbol tables can grow large, and for large numbers I generally
find hex representation preferable of dec. Otoh the other
(pre-existing) message uses dec too ...
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
