On Thu, Mar 31, 2022 at 08:42:47AM +0200, Jan Beulich wrote: > On 30.03.2022 19:04, Roger Pau Monné wrote: > > On Wed, Mar 30, 2022 at 01:05:31PM +0200,>> --- a/xen/arch/x86/livepatch.c > >> +++ b/xen/arch/x86/livepatch.c > >> @@ -157,9 +157,15 @@ void noinline arch_livepatch_apply(struc > >> * loaded hotpatch (to avoid racing against other fixups > >> adding/removing > >> * ENDBR64 or similar instructions). > >> */ > >> - if ( is_endbr64(old_ptr) || is_endbr64_poison(func->old_addr) ) > >> + if ( len >= ENDBR64_LEN && > > > > Sorry, didn't realize before, but shouldn't this check be using > > old_size instead of len (which is based on new_size)? > > Yes and no: In principle yes, but with len == func->new_size in the NOP > case, and with arch_livepatch_verify_func() guaranteeing new_size <= > old_size, the check is still fine for that case. Plus: If new_size was > less than 4 _but_ there's an ENDBR64 at the start, what would we do? I > think there's more that needs fixing in this regard. So I guess I'll > make a v3 with this extra fix dropped and with the livepatch_insn_len() > invocation simply moved. After all the primary goal is to get the > stable trees unstuck.
Right, I agree to try and get the stable trees unblocked ASAP. I think the check for ENDBR is only relevant when we are patching the function with a jump, otherwise the new replacement code should contain the ENDBR instruction already? Thanks, Roger.
