linux-kernel - Re: [PATCH] x86/paravirt: Use relative reference for original instruction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aa4c49a6-65f2-d04c-ee0d-afb9e1262dea@suse.com>
Date:   Thu, 24 Nov 2022 11:18:52 +0100
From:   Juergen Gross <jgross@...e.com>
To:     Hou Wenlong <houwenlong.hwl@...group.com>,
        linux-kernel@...r.kernel.org
Cc:     "Srivatsa S. Bhat (VMware)" <srivatsa@...il.mit.edu>,
        Alexey Makhalov <amakhalov@...are.com>,
        VMware PV-Drivers Reviewers <pv-drivers@...are.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Kees Cook <keescook@...omium.org>, Song Liu <song@...nel.org>,
        Nadav Amit <namit@...are.com>,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH] x86/paravirt: Use relative reference for original
 instruction

On 24.11.22 09:51, Hou Wenlong wrote:
> Similar to the alternative patching, use relative reference for original
> instruction rather than absolute one, which saves 8 bytes for one entry
> on x86_64.  And it could generate R_X86_64_PC32 relocation instead of

8 bytes saved? I think those are 4 bytes only.

> R_X86_64_64 relocation, which also reduces relocation metadata on
> relocatable builds.
> 
> Signed-off-by: Hou Wenlong <houwenlong.hwl@...group.com>
> ---
>   arch/x86/include/asm/paravirt.h       | 6 +++---
>   arch/x86/include/asm/paravirt_types.h | 4 ++--
>   arch/x86/kernel/alternative.c         | 8 +++++---
>   3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index 2851bc2339d5..2cbe9b64e103 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -735,13 +735,13 @@ extern void default_banner(void);
>   
>   #else  /* __ASSEMBLY__ */
>   
> -#define _PVSITE(ptype, ops, word, algn)		\
> +#define _PVSITE(ptype, ops, algn)		\

Would you please drop the algn parameter, too? It isn't needed anymore
as the alignment can be hard coded to be 4 now. This would need to be
adjusted in the _paravirt_alt() macro, too.

>   771:;						\
>   	ops;					\
>   772:;						\
>   	.pushsection .parainstructions,"a";	\
>   	 .align	algn;				\
> -	 word 771b;				\
> +	 .long 771b-.;				\
>   	 .byte ptype;				\
>   	 .byte 772b-771b;			\
>   	 _ASM_ALIGN;				\
> @@ -752,7 +752,7 @@ extern void default_banner(void);
>   #ifdef CONFIG_PARAVIRT_XXL
>   
>   #define PARA_PATCH(off)		((off) / 8)
> -#define PARA_SITE(ptype, ops)	_PVSITE(ptype, ops, .quad, 8)
> +#define PARA_SITE(ptype, ops)	_PVSITE(ptype, ops, 8)
>   #define PARA_INDIRECT(addr)	*addr(%rip)
>   
>   #ifdef CONFIG_DEBUG_ENTRY
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 8c1da419260f..19f709cf7df9 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -5,7 +5,7 @@
>   #ifndef __ASSEMBLY__
>   /* These all sit in the .parainstructions section to tell us what to patch. */
>   struct paravirt_patch_site {
> -	u8 *instr;		/* original instructions */
> +	s32 instr_offset;	/* original instructions */
>   	u8 type;		/* type of this instruction */
>   	u8 len;			/* length of original instruction */
>   };
> @@ -274,7 +274,7 @@ extern struct paravirt_patch_template pv_ops;
>   	"771:\n\t" insn_string "\n" "772:\n"		\
>   	".pushsection .parainstructions,\"a\"\n"	\
>   	_ASM_ALIGN "\n"					\
> -	_ASM_PTR " 771b\n"				\
> +	"  .long 771b-.\n"				\
>   	"  .byte " type "\n"				\
>   	"  .byte 772b-771b\n"				\
>   	_ASM_ALIGN "\n"					\
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 111b809f0ac2..6eea563a098d 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -1232,20 +1232,22 @@ void __init_or_module apply_paravirt(struct paravirt_patch_site *start,
>   {
>   	struct paravirt_patch_site *p;
>   	char insn_buff[MAX_PATCH_LEN];
> +	u8 *instr;
>   
>   	for (p = start; p < end; p++) {
>   		unsigned int used;
>   
> +		instr = (u8 *)&p->instr_offset + p->instr_offset;
>   		BUG_ON(p->len > MAX_PATCH_LEN);
>   		/* prep the buffer with the original instructions */
> -		memcpy(insn_buff, p->instr, p->len);
> -		used = paravirt_patch(p->type, insn_buff, (unsigned long)p->instr, p->len);
> +		memcpy(insn_buff, instr, p->len);
> +		used = paravirt_patch(p->type, insn_buff, (unsigned long)instr, p->len);
>   
>   		BUG_ON(used > p->len);
>   
>   		/* Pad the rest with nops */
>   		add_nops(insn_buff + used, p->len - used);
> -		text_poke_early(p->instr, insn_buff, p->len);
> +		text_poke_early(instr, insn_buff, p->len);
>   	}
>   }
>   extern struct paravirt_patch_site __start_parainstructions[],


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)