Re: [PATCH v3] livepatch: account for patch offset when applying NOP patch

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Mar 31, 2022 at 08:49:46AM +0200, Jan Beulich wrote:
> While not triggered by the trivial xen_nop in-tree patch on
> staging/master, that patch exposes a problem on the stable trees, where
> all functions have ENDBR inserted. When NOP-ing out a range, we need to
> account for this. Handle this right in livepatch_insn_len().
> 
> This requires livepatch_insn_len() to be called _after_ ->patch_offset
> was set.
> 
> Fixes: 6974c75180f1 ("xen/x86: Livepatch: support patching CET-enhanced 
> functions")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Albeit I don't think I understand how the in-place patching is done. I
would expect the !func->new_addr branch of the if in
arch_livepatch_apply to fill the insn buffer with the in-place
replacement instructions, but I only see the buffer getting filled
with nops. I'm likely missing something (not that this patch changes
any of this).

I'm also having trouble figuring out how we assert that the len value
(which is derived from new_size if !new_addr) is not greater than
LIVEPATCH_OPAQUE_SIZE, which is the limit of the insn buffer. Maybe
that's already checked elsewhere.

Thanks, Roger.