From: Juergen Gross <jgross@suse.com> To: Josh Poimboeuf <jpoimboe@redhat.com> Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Andy Lutomirski <luto@kernel.org>, Linus Torvalds <torvalds@linux-foundation.org>, Sasha Levin <alexander.levin@verizon.com>, live-patching@vger.kernel.org, Jiri Slaby <jslaby@suse.cz>, Ingo Molnar <mingo@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>, Peter Zijlstra <peterz@infradead.org>, Mike Galbraith <efault@gmx.de>, Chris Wright <chrisw@sous-sol.org>, Alok Kataria <akataria@vmware.com>, Rusty Russell <rusty@rustcorp.com.au>, virtualization@lists.linux-foundation.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>, xen-devel@lists.xenproject.org, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de> Subject: Re: [PATCH 10/13] x86/alternative: Support indirect call replacement Date: Fri, 17 Nov 2017 06:46:59 +0100 [thread overview] Message-ID: <360d5f88-d840-837a-4520-81bc087a9444@suse.com> (raw) In-Reply-To: <20171116211929.g4q5wa52sq64nhe5@treble> On 16/11/17 22:19, Josh Poimboeuf wrote: > On Wed, Oct 25, 2017 at 01:25:02PM +0200, Juergen Gross wrote: >> On 04/10/17 17:58, Josh Poimboeuf wrote: >>> Add alternative patching support for replacing an instruction with an >>> indirect call. This will be needed for the paravirt alternatives. >>> >>> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> >>> --- >>> arch/x86/kernel/alternative.c | 22 +++++++++++++++------- >>> 1 file changed, 15 insertions(+), 7 deletions(-) >>> >>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c >>> index 3344d3382e91..81c577c7deba 100644 >>> --- a/arch/x86/kernel/alternative.c >>> +++ b/arch/x86/kernel/alternative.c >>> @@ -410,20 +410,28 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, >>> insnbuf_sz = a->replacementlen; >>> >>> /* >>> - * 0xe8 is a relative jump; fix the offset. >>> - * >>> - * Instruction length is checked before the opcode to avoid >>> - * accessing uninitialized bytes for zero-length replacements. >>> + * Fix the address offsets for call and jump instructions which >>> + * use PC-relative addressing. >>> */ >>> if (a->replacementlen == 5 && *insnbuf == 0xe8) { >>> + /* direct call */ >>> *(s32 *)(insnbuf + 1) += replacement - instr; >>> - DPRINTK("Fix CALL offset: 0x%x, CALL 0x%lx", >>> + DPRINTK("Fix direct CALL offset: 0x%x, CALL 0x%lx", >>> *(s32 *)(insnbuf + 1), >>> (unsigned long)instr + *(s32 *)(insnbuf + 1) + 5); >>> - } >>> >>> - if (a->replacementlen && is_jmp(replacement[0])) >>> + } else if (a->replacementlen == 6 && *insnbuf == 0xff && >>> + *(insnbuf+1) == 0x15) { >>> + /* indirect call */ >>> + *(s32 *)(insnbuf + 2) += replacement - instr; >>> + DPRINTK("Fix indirect CALL offset: 0x%x, CALL *0x%lx", >>> + *(s32 *)(insnbuf + 2), >>> + (unsigned long)instr + *(s32 *)(insnbuf + 2) + 6); >>> + >>> + } else if (a->replacementlen && is_jmp(replacement[0])) { >> >> Is this correct? Without your patch this was: >> >> if (*insnbuf == 0xe8) ... >> if (is_jmp(replacement[0])) ... >> >> Now you have: >> >> if (*insnbuf == 0xe8) ... >> else if (*insnbuf == 0xff15) ... >> else if (is_jmp(replacement[0])) ... >> >> So only one or none of the three variants will be executed. In the past >> it could be none, one or both. > > It can't be a call *and* a jump. It's either one or the other. > > Maybe it's a little confusing that the jump check uses replacement[0] > while the other checks use *insnbuf? They have the same value, so the Right, I was fooled by that. > same variable should probably be used everywhere for consistency. I can > make them more consistent. > I'd appreciate that. :-) Juergen
WARNING: multiple messages have this Message-ID (diff)
From: Juergen Gross <jgross@suse.com> To: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>, Rusty Russell <rusty@rustcorp.com.au>, Mike Galbraith <efault@gmx.de>, xen-devel@lists.xenproject.org, Peter Zijlstra <peterz@infradead.org>, Jiri Slaby <jslaby@suse.cz>, x86@kernel.org, linux-kernel@vger.kernel.org, Sasha Levin <alexander.levin@verizon.com>, Chris Wright <chrisw@sous-sol.org>, Thomas Gleixner <tglx@linutronix.de>, Andy Lutomirski <luto@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@alien8.de>, live-patching@vger.kernel.org, Alok Kataria <akataria@vmware.com>, virtualization@lists.linux-foundation.org, Linus Torvalds <torvalds@linux-foundation.org>, Ingo Molnar <mingo@kernel.org> Subject: Re: [PATCH 10/13] x86/alternative: Support indirect call replacement Date: Fri, 17 Nov 2017 06:46:59 +0100 [thread overview] Message-ID: <360d5f88-d840-837a-4520-81bc087a9444@suse.com> (raw) In-Reply-To: <20171116211929.g4q5wa52sq64nhe5@treble> On 16/11/17 22:19, Josh Poimboeuf wrote: > On Wed, Oct 25, 2017 at 01:25:02PM +0200, Juergen Gross wrote: >> On 04/10/17 17:58, Josh Poimboeuf wrote: >>> Add alternative patching support for replacing an instruction with an >>> indirect call. This will be needed for the paravirt alternatives. >>> >>> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> >>> --- >>> arch/x86/kernel/alternative.c | 22 +++++++++++++++------- >>> 1 file changed, 15 insertions(+), 7 deletions(-) >>> >>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c >>> index 3344d3382e91..81c577c7deba 100644 >>> --- a/arch/x86/kernel/alternative.c >>> +++ b/arch/x86/kernel/alternative.c >>> @@ -410,20 +410,28 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, >>> insnbuf_sz = a->replacementlen; >>> >>> /* >>> - * 0xe8 is a relative jump; fix the offset. >>> - * >>> - * Instruction length is checked before the opcode to avoid >>> - * accessing uninitialized bytes for zero-length replacements. >>> + * Fix the address offsets for call and jump instructions which >>> + * use PC-relative addressing. >>> */ >>> if (a->replacementlen == 5 && *insnbuf == 0xe8) { >>> + /* direct call */ >>> *(s32 *)(insnbuf + 1) += replacement - instr; >>> - DPRINTK("Fix CALL offset: 0x%x, CALL 0x%lx", >>> + DPRINTK("Fix direct CALL offset: 0x%x, CALL 0x%lx", >>> *(s32 *)(insnbuf + 1), >>> (unsigned long)instr + *(s32 *)(insnbuf + 1) + 5); >>> - } >>> >>> - if (a->replacementlen && is_jmp(replacement[0])) >>> + } else if (a->replacementlen == 6 && *insnbuf == 0xff && >>> + *(insnbuf+1) == 0x15) { >>> + /* indirect call */ >>> + *(s32 *)(insnbuf + 2) += replacement - instr; >>> + DPRINTK("Fix indirect CALL offset: 0x%x, CALL *0x%lx", >>> + *(s32 *)(insnbuf + 2), >>> + (unsigned long)instr + *(s32 *)(insnbuf + 2) + 6); >>> + >>> + } else if (a->replacementlen && is_jmp(replacement[0])) { >> >> Is this correct? Without your patch this was: >> >> if (*insnbuf == 0xe8) ... >> if (is_jmp(replacement[0])) ... >> >> Now you have: >> >> if (*insnbuf == 0xe8) ... >> else if (*insnbuf == 0xff15) ... >> else if (is_jmp(replacement[0])) ... >> >> So only one or none of the three variants will be executed. In the past >> it could be none, one or both. > > It can't be a call *and* a jump. It's either one or the other. > > Maybe it's a little confusing that the jump check uses replacement[0] > while the other checks use *insnbuf? They have the same value, so the Right, I was fooled by that. > same variable should probably be used everywhere for consistency. I can > make them more consistent. > I'd appreciate that. :-) Juergen
next prev parent reply other threads:[~2017-11-17 5:47 UTC|newest] Thread overview: 175+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-04 15:58 [PATCH 00/13] x86/paravirt: Make pv ops code generation more closely match reality Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 01/13] x86/paravirt: remove wbinvd() paravirt interface Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-24 13:17 ` Juergen Gross 2017-10-24 13:17 ` Juergen Gross 2017-10-24 13:17 ` Juergen Gross 2017-11-17 14:39 ` Borislav Petkov 2017-11-17 14:39 ` Borislav Petkov 2017-11-17 14:39 ` Borislav Petkov 2017-10-04 15:58 ` [PATCH 02/13] x86/paravirt: Fix output constraint macro names Josh Poimboeuf 2017-10-25 9:33 ` Juergen Gross 2017-10-25 9:33 ` Juergen Gross 2017-11-16 20:50 ` Josh Poimboeuf 2017-11-16 20:50 ` Josh Poimboeuf 2017-11-16 20:50 ` Josh Poimboeuf 2017-11-17 6:55 ` Juergen Gross 2017-11-17 6:55 ` Juergen Gross 2017-11-17 6:55 ` Juergen Gross 2017-10-25 9:33 ` Juergen Gross 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 03/13] x86/paravirt: Convert native patch assembly code strings to macros Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 9:46 ` Juergen Gross 2017-10-25 9:46 ` Juergen Gross 2017-10-25 9:46 ` Juergen Gross 2017-11-16 21:04 ` Josh Poimboeuf 2017-11-16 21:04 ` Josh Poimboeuf 2017-11-16 21:04 ` Josh Poimboeuf 2017-11-17 18:07 ` Borislav Petkov 2017-11-17 18:07 ` Borislav Petkov 2017-11-17 18:07 ` Borislav Petkov 2017-11-17 19:10 ` Juergen Gross 2017-11-17 19:10 ` Juergen Gross 2017-11-17 19:10 ` Juergen Gross 2017-11-17 19:42 ` Josh Poimboeuf 2017-11-17 19:42 ` Josh Poimboeuf 2017-11-18 10:20 ` Juergen Gross 2017-11-18 10:20 ` Juergen Gross 2017-11-18 13:17 ` Josh Poimboeuf 2017-11-18 13:17 ` Josh Poimboeuf 2017-11-18 13:17 ` Josh Poimboeuf 2017-11-18 10:20 ` Juergen Gross 2017-11-17 19:42 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 04/13] x86/paravirt: Convert DEF_NATIVE macro to GCC extended asm syntax Josh Poimboeuf 2017-10-25 10:03 ` Juergen Gross 2017-10-25 10:03 ` Juergen Gross 2017-10-25 10:03 ` Juergen Gross 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 05/13] x86/paravirt: Move paravirt asm macros to paravirt-asm.h Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 10:32 ` Juergen Gross 2017-10-25 10:32 ` Juergen Gross 2017-10-25 10:32 ` Juergen Gross 2017-10-04 15:58 ` [PATCH 06/13] x86/paravirt: Clean up paravirt-asm.h Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 10:59 ` Juergen Gross 2017-10-25 10:59 ` Juergen Gross 2017-10-25 10:59 ` Juergen Gross 2017-10-04 15:58 ` [PATCH 07/13] x86/paravirt: Simplify ____PVOP_CALL() Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 11:01 ` Juergen Gross 2017-10-25 11:01 ` Juergen Gross 2017-10-25 11:01 ` Juergen Gross 2017-11-22 16:35 ` Borislav Petkov 2017-11-22 16:35 ` Borislav Petkov 2017-11-22 16:35 ` Borislav Petkov 2017-10-04 15:58 ` [PATCH 08/13] x86/paravirt: Clean up paravirt_types.h Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 11:08 ` Juergen Gross 2017-10-25 11:08 ` Juergen Gross 2017-10-25 11:08 ` Juergen Gross 2017-11-22 20:46 ` Borislav Petkov 2017-11-22 20:46 ` Borislav Petkov 2017-11-22 20:46 ` Borislav Petkov 2017-10-04 15:58 ` [PATCH 09/13] x86/asm: Convert ALTERNATIVE*() assembler macros to preprocessor macros Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 11:14 ` Juergen Gross 2017-10-25 11:14 ` Juergen Gross 2017-10-25 11:14 ` Juergen Gross 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 10/13] x86/alternative: Support indirect call replacement Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-25 11:25 ` Juergen Gross 2017-10-25 11:25 ` Juergen Gross 2017-10-25 11:25 ` Juergen Gross 2017-11-16 21:19 ` Josh Poimboeuf 2017-11-16 21:19 ` Josh Poimboeuf 2017-11-16 21:19 ` Josh Poimboeuf 2017-11-17 5:46 ` Juergen Gross 2017-11-17 5:46 ` Juergen Gross [this message] 2017-11-17 5:46 ` Juergen Gross 2017-11-17 19:52 ` H. Peter Anvin 2017-11-17 19:52 ` H. Peter Anvin 2017-11-17 19:52 ` H. Peter Anvin 2017-10-04 15:58 ` [PATCH 11/13] x86/paravirt: Add paravirt alternatives infrastructure Josh Poimboeuf 2017-10-05 20:35 ` Boris Ostrovsky 2017-10-05 20:35 ` Boris Ostrovsky 2017-10-06 14:32 ` Josh Poimboeuf 2017-10-06 15:29 ` Boris Ostrovsky 2017-10-06 15:29 ` Boris Ostrovsky 2017-10-06 16:30 ` Josh Poimboeuf 2017-10-06 16:30 ` Josh Poimboeuf 2017-10-06 16:30 ` Josh Poimboeuf 2017-10-06 15:29 ` Boris Ostrovsky 2017-10-12 19:11 ` Boris Ostrovsky 2017-10-12 19:11 ` Boris Ostrovsky 2017-10-12 19:11 ` Boris Ostrovsky 2017-10-12 19:27 ` Andrew Cooper 2017-10-12 19:27 ` [Xen-devel] " Andrew Cooper 2017-10-12 19:27 ` Andrew Cooper 2017-10-12 19:53 ` Boris Ostrovsky 2017-10-12 19:53 ` [Xen-devel] " Boris Ostrovsky 2017-10-12 19:53 ` Boris Ostrovsky 2017-10-16 18:18 ` Boris Ostrovsky 2017-10-16 18:18 ` [Xen-devel] " Boris Ostrovsky 2017-10-16 18:18 ` Boris Ostrovsky 2017-10-17 5:24 ` Josh Poimboeuf 2017-10-17 5:24 ` Josh Poimboeuf 2017-10-17 13:58 ` Boris Ostrovsky 2017-10-17 13:58 ` Boris Ostrovsky 2017-10-17 14:36 ` Josh Poimboeuf 2017-10-17 14:36 ` [Xen-devel] " Josh Poimboeuf 2017-10-17 14:36 ` Josh Poimboeuf 2017-10-17 15:36 ` Boris Ostrovsky 2017-10-17 15:36 ` [Xen-devel] " Boris Ostrovsky 2017-10-17 15:36 ` Boris Ostrovsky 2017-10-17 20:17 ` Josh Poimboeuf 2017-10-17 20:17 ` Josh Poimboeuf 2017-10-17 20:36 ` Boris Ostrovsky 2017-10-17 20:36 ` Boris Ostrovsky 2017-10-17 20:50 ` Josh Poimboeuf 2017-10-17 20:50 ` [Xen-devel] " Josh Poimboeuf 2017-10-17 20:59 ` Boris Ostrovsky 2017-10-17 20:59 ` Boris Ostrovsky 2017-10-17 21:03 ` Josh Poimboeuf 2017-10-17 21:03 ` [Xen-devel] " Josh Poimboeuf 2017-10-17 21:03 ` Josh Poimboeuf 2017-10-17 20:59 ` Boris Ostrovsky 2017-10-17 20:50 ` [Xen-devel] " Josh Poimboeuf 2017-10-17 20:36 ` Boris Ostrovsky 2017-10-17 20:17 ` Josh Poimboeuf 2017-10-17 13:58 ` Boris Ostrovsky 2017-10-17 5:24 ` Josh Poimboeuf 2017-10-17 13:10 ` [Xen-devel] " Brian Gerst 2017-10-17 13:10 ` Brian Gerst 2017-10-17 13:10 ` [Xen-devel] " Brian Gerst 2017-10-17 14:05 ` Boris Ostrovsky 2017-10-17 14:05 ` [Xen-devel] " Boris Ostrovsky 2017-10-17 14:05 ` Boris Ostrovsky 2017-10-06 14:32 ` Josh Poimboeuf 2017-10-06 14:32 ` Josh Poimboeuf 2017-10-05 20:35 ` Boris Ostrovsky 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 12/13] objtool: Add support for new .pv_altinstructions section Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` [PATCH 13/13] x86/paravirt: Convert natively patched pv ops to use paravirt alternatives Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-04 15:58 ` Josh Poimboeuf 2017-10-06 7:35 ` [Xen-devel] [PATCH 00/13] x86/paravirt: Make pv ops code generation more closely match reality Vitaly Kuznetsov 2017-10-06 7:35 ` Vitaly Kuznetsov 2017-10-06 14:36 ` Josh Poimboeuf 2017-10-06 14:36 ` Josh Poimboeuf 2017-10-06 14:36 ` [Xen-devel] " Josh Poimboeuf 2017-10-06 7:35 ` Vitaly Kuznetsov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=360d5f88-d840-837a-4520-81bc087a9444@suse.com \ --to=jgross@suse.com \ --cc=akataria@vmware.com \ --cc=alexander.levin@verizon.com \ --cc=boris.ostrovsky@oracle.com \ --cc=bp@alien8.de \ --cc=chrisw@sous-sol.org \ --cc=efault@gmx.de \ --cc=hpa@zytor.com \ --cc=jpoimboe@redhat.com \ --cc=jslaby@suse.cz \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=luto@kernel.org \ --cc=mingo@kernel.org \ --cc=peterz@infradead.org \ --cc=rusty@rustcorp.com.au \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=virtualization@lists.linux-foundation.org \ --cc=x86@kernel.org \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.