Re: [PATCH v3 2/2] xen/x86: Livepatch: support patching CET-enhanced functions

["Doebel, Bjoern" <doebel@xxxxxxxxx>]

On 08.03.22 14:06, Konrad Rzeszutek Wilk wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you can confirm the sender and know the 
> content is safe.
>
>
>
> On Tue, Mar 08, 2022 at 12:44:54PM +0000, Andrew Cooper wrote:
> > On 08/03/2022 10:29, Bjoern Doebel wrote:
> > > @@ -104,18 +122,34 @@ void noinline arch_livepatch_revive(void)
> > >
> > >   int arch_livepatch_verify_func(const struct livepatch_func *func)
> > >   {
> > > +    BUILD_BUG_ON(sizeof(struct x86_livepatch_meta) != LIVEPATCH_OPAQUE_SIZE);
> > > +
> > >       /* If NOPing.. */
> > >       if ( !func->new_addr )
> > >       {
> > >           /* Only do up to maximum amount we can put in the ->opaque. */
> > > -        if ( func->new_size > sizeof(func->opaque) )
> > > +        if ( func->new_size > sizeof_field(struct x86_livepatch_meta,
> > > +                                           instruction) )
> > >               return -EOPNOTSUPP;
> > >
> > >           if ( func->old_size < func->new_size )
> > >               return -EINVAL;
> > >       }
> > > -    else if ( func->old_size < ARCH_PATCH_INSN_SIZE )
> > > -        return -EINVAL;
> > > +    else
> > > +    {
> > > +        /*
> > > +         * Space needed now depends on whether the target function
> > > +         * starts with an ENDBR64 instruction.
> > > +         */
> > > +        uint8_t needed;
> > > +
> > > +        needed = ARCH_PATCH_INSN_SIZE;
> > > +        if ( is_endbr64(func->old_addr) )
> > > +            needed += ENDBR64_LEN;
> > This won't work for cf_clobber targets, I don't think.  The ENDBR gets
> > converted to NOP4 and fails this check, but the altcalls calling
> > old_func had their displacements adjusted by +4.
> >
> > The is_endbr64() check will fail, and the 5-byte jmp will be written at
> > the start of the function, and corrupt the instruction stream for the
> > altcall()'d callers.
> >
> > Let me write an incremental patch to help.
> Please add Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> on the patches.
Thanks, will do!

Bjoern

> Thank you
> > ~Andrew


Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879