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

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

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.
> I'm not sure here: Are all symbols starting with $t to be rejected, or
> only $t but not e.g. $txyz? According to some other code I have
> lying around it ought to be "$t" and any symbols starting with "$t.",
> and would be in line with patch 6. But I guess the ARM maintainers
> will know best.
Only $t and $t.* should be rejected. All the others may be valid in the 
future.
Looking at the spec, I am wondering if we should also check the type and 
binding of the symbols. I have the impression that the naming is 
specific to STT_NOTYPE and STB_LOCAL. Any opinions?
Similar question for the previous patch (#6).

Regards,

--
Julien Grall

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