[PATCH 1/5] x86/alternative: simplify DUMP_BYTES macro

[PATCH 1/5] x86/alternative: simplify DUMP_BYTES macro

[Alexey Dobriyan]

Avoid zero length check with clever whitespace placement in the format
string.

Signed-off-by: Alexey Dobriyan (CloudLinux) <adobriyan@gmail.com>
---
 arch/x86/kernel/alternative.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5007c3ffe96f..6c9758ee6810 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -66,13 +66,10 @@ do {									\
 	if (unlikely(debug_alternative)) {				\
 		int j;							\
 									\
-		if (!(len))						\
-			break;						\
-									\
 		printk(KERN_DEBUG pr_fmt(fmt), ##args);			\
-		for (j = 0; j < (len) - 1; j++)				\
-			printk(KERN_CONT "%02hhx ", buf[j]);		\
-		printk(KERN_CONT "%02hhx\n", buf[j]);			\
+		for (j = 0; j < (len); j++)				\
+			printk(KERN_CONT " %02hhx", buf[j]);		\
+		printk(KERN_CONT "\n");					\
 	}								\
 } while (0)
 
@@ -214,7 +211,7 @@ static __always_inline int optimize_nops_range(u8 *instr, u8 instrlen, int off)
 	add_nops(instr + off, nnops);
 	local_irq_restore(flags);
 
-	DUMP_BYTES(instr, instrlen, "%px: [%d:%d) optimized NOPs: ", instr, off, i);
+	DUMP_BYTES(instr, instrlen, "%px: [%d:%d) optimized NOPs:", instr, off, i);
 
 	return nnops;
 }
@@ -303,8 +300,8 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start,
 			instr, instr, a->instrlen,
 			replacement, a->replacementlen);
 
-		DUMP_BYTES(instr, a->instrlen, "%px:   old_insn: ", instr);
-		DUMP_BYTES(replacement, a->replacementlen, "%px:   rpl_insn: ", replacement);
+		DUMP_BYTES(instr, a->instrlen, "%px:   old_insn:", instr);
+		DUMP_BYTES(replacement, a->replacementlen, "%px:   rpl_insn:", replacement);
 
 		memcpy(insn_buff, replacement, a->replacementlen);
 		insn_buff_sz = a->replacementlen;
@@ -328,7 +325,7 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start,
 		for (; insn_buff_sz < a->instrlen; insn_buff_sz++)
 			insn_buff[insn_buff_sz] = 0x90;
 
-		DUMP_BYTES(insn_buff, insn_buff_sz, "%px: final_insn: ", instr);
+		DUMP_BYTES(insn_buff, insn_buff_sz, "%px: final_insn:", instr);
 
 		text_poke_early(instr, insn_buff, insn_buff_sz);
 
@@ -499,8 +496,8 @@ void __init_or_module noinline apply_retpolines(s32 *start, s32 *end)
 		len = patch_retpoline(addr, &insn, bytes);
 		if (len == insn.length) {
 			optimize_nops(bytes, len);
-			DUMP_BYTES(((u8*)addr),  len, "%px: orig: ", addr);
-			DUMP_BYTES(((u8*)bytes), len, "%px: repl: ", addr);
+			DUMP_BYTES(((u8*)addr),  len, "%px: orig:", addr);
+			DUMP_BYTES(((u8*)bytes), len, "%px: repl:", addr);
 			text_poke_early(addr, bytes, len);
 		}
 	}
-- 
2.34.1

[PATCH 2/5] x86/alternative: bump MAX_PATCH_LEN

[Alexey Dobriyan]

Replacement instructions are raw data, there is no need to reserve
space for the NUL terminator.

Signed-off-by: Alexey Dobriyan (CloudLinux) <adobriyan@gmail.com>
---
 arch/x86/kernel/alternative.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 6c9758ee6810..1e6fd814dd08 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -35,7 +35,7 @@ int __read_mostly alternatives_patched;
 
 EXPORT_SYMBOL_GPL(alternatives_patched);
 
-#define MAX_PATCH_LEN (255-1)
+#define MAX_PATCH_LEN 255
 
 static int __initdata_or_module debug_alternative;
 
-- 
2.34.1

[PATCH 3/5] x86/alternative: record .altinstructions section entity size

[Alexey Dobriyan]

.altinstructions entry was 12 bytes in size, then it was 13 bytes,
now it is 12 again. It was 24 bytes on some distros as well.
Record this information as section sh_entsize value so that tools
which parse .altinstructions have easier time.

Signed-off-by: Alexey Dobriyan (CloudLinux) <adobriyan@gmail.com>
---
 arch/x86/include/asm/alternative.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 58eee6402832..cf7722a106b3 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -9,6 +9,8 @@
 #define ALTINSTR_FLAG_INV	(1 << 15)
 #define ALT_NOT(feat)		((feat) | ALTINSTR_FLAG_INV)
 
+#define sizeof_struct_alt_instr 12
+
 #ifndef __ASSEMBLY__
 
 #include <linux/stddef.h>
@@ -66,6 +68,7 @@ struct alt_instr {
 	u8  instrlen;		/* length of original instruction */
 	u8  replacementlen;	/* length of new instruction */
 } __packed;
+_Static_assert(sizeof(struct alt_instr) == sizeof_struct_alt_instr, "");
 
 /*
  * Debug flag that can be tested to see whether alternative
@@ -159,7 +162,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 /* alternative assembly primitive: */
 #define ALTERNATIVE(oldinstr, newinstr, feature)			\
 	OLDINSTR(oldinstr, 1)						\
-	".pushsection .altinstructions,\"a\"\n"				\
+	".pushsection .altinstructions,\"aM\",@progbits," __stringify(sizeof_struct_alt_instr) "\n"\
 	ALTINSTR_ENTRY(feature, 1)					\
 	".popsection\n"							\
 	".pushsection .altinstr_replacement, \"ax\"\n"			\
@@ -168,7 +171,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 
 #define ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2)\
 	OLDINSTR_2(oldinstr, 1, 2)					\
-	".pushsection .altinstructions,\"a\"\n"				\
+	".pushsection .altinstructions,\"aM\",@progbits," __stringify(sizeof_struct_alt_instr) "\n"\
 	ALTINSTR_ENTRY(feature1, 1)					\
 	ALTINSTR_ENTRY(feature2, 2)					\
 	".popsection\n"							\
@@ -184,7 +187,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 
 #define ALTERNATIVE_3(oldinsn, newinsn1, feat1, newinsn2, feat2, newinsn3, feat3) \
 	OLDINSTR_3(oldinsn, 1, 2, 3)						\
-	".pushsection .altinstructions,\"a\"\n"					\
+	".pushsection .altinstructions,\"aM\",@progbits," __stringify(sizeof_struct_alt_instr) "\n"\
 	ALTINSTR_ENTRY(feat1, 1)						\
 	ALTINSTR_ENTRY(feat2, 2)						\
 	ALTINSTR_ENTRY(feat3, 3)						\
@@ -331,7 +334,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	.skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90
 142:
 
-	.pushsection .altinstructions,"a"
+	.pushsection .altinstructions,"aM",@progbits,sizeof_struct_alt_instr
 	altinstruction_entry 140b,143f,\feature,142b-140b,144f-143f
 	.popsection
 
@@ -368,7 +371,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 		(alt_max_short(new_len1, new_len2) - (old_len)),0x90
 142:
 
-	.pushsection .altinstructions,"a"
+	.pushsection .altinstructions,"aM",@progbits,sizeof_struct_alt_instr
 	altinstruction_entry 140b,143f,\feature1,142b-140b,144f-143f
 	altinstruction_entry 140b,144f,\feature2,142b-140b,145f-144f
 	.popsection
-- 
2.34.1

[PATCH 4/5] x86/alternative: make .altinstr_replacement section non-executable

[Alexey Dobriyan]

Instructions in .altinstr_replacement section aren't meant to be
executed by CPU directly, only after being copied to the real
instruction stream in .text/.init_text.

Signed-off-by: Alexey Dobriyan (CloudLinux) <adobriyan@gmail.com>
---
 arch/x86/include/asm/alternative.h | 10 +++++-----
 tools/objtool/check.c              |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index cf7722a106b3..d754a18edc99 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -165,7 +165,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	".pushsection .altinstructions,\"aM\",@progbits," __stringify(sizeof_struct_alt_instr) "\n"\
 	ALTINSTR_ENTRY(feature, 1)					\
 	".popsection\n"							\
-	".pushsection .altinstr_replacement, \"ax\"\n"			\
+	".pushsection .altinstr_replacement, \"a\"\n"			\
 	ALTINSTR_REPLACEMENT(newinstr, 1)				\
 	".popsection\n"
 
@@ -175,7 +175,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	ALTINSTR_ENTRY(feature1, 1)					\
 	ALTINSTR_ENTRY(feature2, 2)					\
 	".popsection\n"							\
-	".pushsection .altinstr_replacement, \"ax\"\n"			\
+	".pushsection .altinstr_replacement, \"a\"\n"			\
 	ALTINSTR_REPLACEMENT(newinstr1, 1)				\
 	ALTINSTR_REPLACEMENT(newinstr2, 2)				\
 	".popsection\n"
@@ -192,7 +192,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	ALTINSTR_ENTRY(feat2, 2)						\
 	ALTINSTR_ENTRY(feat3, 3)						\
 	".popsection\n"								\
-	".pushsection .altinstr_replacement, \"ax\"\n"				\
+	".pushsection .altinstr_replacement, \"a\"\n"				\
 	ALTINSTR_REPLACEMENT(newinsn1, 1)					\
 	ALTINSTR_REPLACEMENT(newinsn2, 2)					\
 	ALTINSTR_REPLACEMENT(newinsn3, 3)					\
@@ -338,7 +338,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	altinstruction_entry 140b,143f,\feature,142b-140b,144f-143f
 	.popsection
 
-	.pushsection .altinstr_replacement,"ax"
+	.pushsection .altinstr_replacement,"a"
 143:
 	\newinstr
 144:
@@ -376,7 +376,7 @@ static inline int alternatives_text_reserved(void *start, void *end)
 	altinstruction_entry 140b,144f,\feature2,142b-140b,145f-144f
 	.popsection
 
-	.pushsection .altinstr_replacement,"ax"
+	.pushsection .altinstr_replacement,"a"
 143:
 	\newinstr1
 144:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 7c33ec67c4a9..18c32035f41c 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -359,7 +359,7 @@ static int decode_instructions(struct objtool_file *file)
 
 	for_each_sec(file, sec) {
 
-		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+		if (!(sec->sh.sh_flags & SHF_EXECINSTR) && strcmp(sec->name, ".altinstr_replacement"))
 			continue;
 
 		if (strcmp(sec->name, ".altinstr_replacement") &&
-- 
2.34.1

[PATCH 5/5] x86/unwind/orc: delete dead write in __orc_find()

[Alexey Dobriyan]

Also move "mid" variable to the innermost scope and delete useless
parenthesis while I'm at it.

Signed-off-by: Alexey Dobriyan (CloudLinux) <adobriyan@gmail.com>
---
 arch/x86/kernel/unwind_orc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 2de3c8c5eba9..d38125ea1bf6 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -35,7 +35,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
 {
 	int *first = ip_table;
 	int *last = ip_table + num_entries - 1;
-	int *mid = first, *found = first;
+	int *found = first;
 
 	if (!num_entries)
 		return NULL;
@@ -47,7 +47,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
 	 * ignored when they conflict with a real entry.
 	 */
 	while (first <= last) {
-		mid = first + ((last - first) / 2);
+		int *mid = first + (last - first) / 2;
 
 		if (orc_ip(mid) <= ip) {
 			found = mid;
-- 
2.34.1

RE: [PATCH 5/5] x86/unwind/orc: delete dead write in __orc_find()

[David Laight]

From: Alexey Dobriyan
> Sent: 11 March 2022 14:43
> 
> Also move "mid" variable to the innermost scope and delete useless
> parenthesis while I'm at it.

Hiding the definition of 'mid' in the inner scope is pointless.

I'm also not 100% sure that 'found' is always set.
Consider the ip < orc_ip(first) case.

	David

> 
> Signed-off-by: Alexey Dobriyan (CloudLinux) <adobriyan@gmail.com>
> ---
>  arch/x86/kernel/unwind_orc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index 2de3c8c5eba9..d38125ea1bf6 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -35,7 +35,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
>  {
>  	int *first = ip_table;
>  	int *last = ip_table + num_entries - 1;
> -	int *mid = first, *found = first;
> +	int *found = first;
> 
>  	if (!num_entries)
>  		return NULL;
> @@ -47,7 +47,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
>  	 * ignored when they conflict with a real entry.
>  	 */
>  	while (first <= last) {
> -		mid = first + ((last - first) / 2);
> +		int *mid = first + (last - first) / 2;
> 
>  		if (orc_ip(mid) <= ip) {
>  			found = mid;
> --
> 2.34.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)