Skip to contentSkip to conversation

Remove __end_entry_SYSENTER_compat?

PATCH Borislav Petkov 2 participants 3 messages Jul 12, 2017

Subsystems (2)
x86 x86 entry code
lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

Anyone think this is an OK-ish idea?

It saves us the global symbol but requires the two functions to remained glued together. :-\

---

arch/x86/entry/entry_64_compat.S +3 -1
index e1721dafbcb1..262519da8661 100644
@@ -131,7 +131,6 @@ ENTRY(entry_SYSENTER_compat)
 	pushq	$X86_EFLAGS_FIXED
 	popfq
 	jmp	.Lsysenter_flags_fixed
-GLOBAL(__end_entry_SYSENTER_compat)
 ENDPROC(entry_SYSENTER_compat)
 
 /*

@@ -180,6 +179,9 @@ ENDPROC(entry_SYSENTER_compat)
  * edi  arg5
  * esp  user stack
  * 0(%esp) arg6
+ *
+ * DO NOT! move this function and the above before adjusting
+ * is_sysenter_singlestep().
  */
 ENTRY(entry_SYSCALL_compat)
 	/* Interrupts are off on entry. */
arch/x86/include/asm/proto.h +0 -1
index 8d3964fc5f91..afdef9f3f0f0 100644
@@ -21,7 +21,6 @@ void __end_SYSENTER_singlestep_region(void);
 
 #ifdef CONFIG_IA32_EMULATION
 void entry_SYSENTER_compat(void);
-void __end_entry_SYSENTER_compat(void);
 void entry_SYSCALL_compat(void);
 void entry_INT80_compat(void);
 #endif
arch/x86/kernel/traps.c +1 -1
index bf54309b85da..143902ffe9ff 100644
@@ -668,7 +668,7 @@ static bool is_sysenter_singlestep(struct pt_regs *regs)
 		(unsigned long)__begin_SYSENTER_singlestep_region;
 #elif defined(CONFIG_IA32_EMULATION)
 	return (regs->ip - (unsigned long)entry_SYSENTER_compat) <
-		(unsigned long)__end_entry_SYSENTER_compat -
+		(unsigned long)entry_SYSCALL_compat -
 		(unsigned long)entry_SYSENTER_compat;
 #else
 	return false;
Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
#1 lore raw
Quoted 45 lines · Borislav Petkov

Anyone think this is an OK-ish idea?

It saves us the global symbol but requires the two functions to remained glued together. :-\

---

arch/x86/entry/entry_64_compat.S +3 -1
index e1721dafbcb1..262519da8661 100644
@@ -131,7 +131,6 @@ ENTRY(entry_SYSENTER_compat)
 	pushq	$X86_EFLAGS_FIXED
 	popfq
 	jmp	.Lsysenter_flags_fixed
-GLOBAL(__end_entry_SYSENTER_compat)
 ENDPROC(entry_SYSENTER_compat)
 
 /*

@@ -180,6 +179,9 @@ ENDPROC(entry_SYSENTER_compat)
  * edi  arg5
  * esp  user stack
  * 0(%esp) arg6
+ *
+ * DO NOT! move this function and the above before adjusting
+ * is_sysenter_singlestep().
  */
 ENTRY(entry_SYSCALL_compat)
 	/* Interrupts are off on entry. */
arch/x86/include/asm/proto.h +0 -1
index 8d3964fc5f91..afdef9f3f0f0 100644
@@ -21,7 +21,6 @@ void __end_SYSENTER_singlestep_region(void);
 
 #ifdef CONFIG_IA32_EMULATION
 void entry_SYSENTER_compat(void);
-void __end_entry_SYSENTER_compat(void);
 void entry_SYSCALL_compat(void);
 void entry_INT80_compat(void);
 #endif
arch/x86/kernel/traps.c +0 -0
index bf54309b85da..143902ffe9ff 100644

arch/x86/kernel/traps.c static bool is_sysenter_singlestep(struct pt_regs *regs) · line 674

Quoted 8 lines
arch/x86/kernel/traps.c +1 -1
@@ -668,7 +668,7 @@ static bool is_sysenter_singlestep(struct pt_regs *regs)
 		(unsigned long)__begin_SYSENTER_singlestep_region;
 #elif defined(CONFIG_IA32_EMULATION)
 	return (regs->ip - (unsigned long)entry_SYSENTER_compat) <
-		(unsigned long)__end_entry_SYSENTER_compat -
+		(unsigned long)entry_SYSCALL_compat -
 		(unsigned long)entry_SYSENTER_compat;
 #else
 	return false;

Hm, I'd argue that the old code is much clearer: we need both the start and the end of a function and have the properly named symbols for that.

That entry_SYSCALL_compat() happens to start just where __end_entry_SYSENTER_compat is an accident of placement.

Is it even true - doesn't ENTRY() imply an .align, in which case it might be that __end_entry_SYSENTER_compat != entry_SYSCALL_compat?

In fact that appears to be the case for my defconfig:

ffffffff81942f90 T entry_SYSENTER_compat
ffffffff81942feb T __end_entry_SYSENTER_compat
ffffffff81942ff0 T entry_SYSCALL_compat

So unless there's some disadvantage beyond having one more symbol, I'd favor the old approach.

Thanks,

Ingo

author maintainer
#2 lore raw
Quoted 5 lines · Ingo Molnar

Hm, I'd argue that the old code is much clearer: we need both the start and the end of a function and have the properly named symbols for that.

That entry_SYSCALL_compat() happens to start just where __end_entry_SYSENTER_compat is an accident of placement.

Yeah, probably not worth the effort of actually making it less reliable this way...

Quoted 2 lines

Is it even true - doesn't ENTRY() imply an .align, in which case it might be that __end_entry_SYSENTER_compat != entry_SYSCALL_compat?

Yes, ENTRY does .p2align:

.globl __end_entry_SYSENTER_compat; __end_entry_SYSENTER_compat:
.type entry_SYSENTER_compat, @function ; .size entry_SYSENTER_compat, .-entry_SYSENTER_compat # 184 "arch/x86/entry/entry_64_compat.S"
.globl entry_SYSCALL_compat ; .p2align 4, 0x90 ; entry_SYSCALL_compat:

				^^^^^^

Pads to an alignment of 4 with NOPs.

Quoted 8 lines

In fact that appears to be the case for my defconfig:

ffffffff81942f90 T entry_SYSENTER_compat
ffffffff81942feb T __end_entry_SYSENTER_compat
ffffffff81942ff0 T entry_SYSCALL_compat

So unless there's some disadvantage beyond having one more symbol, I'd favor the old approach.

Yeah, my only concern was getting rid of the global symbol but it doesn't work in this case.

Thanks.

Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--