Skip to content

Commit

Permalink
x86/entry: Unbreak __irqentry_text_start/end magic
Browse files Browse the repository at this point in the history
The entry rework moved interrupt entry code from the irqentry to the
noinstr section which made the irqentry section empty.

This breaks boundary checks which rely on the __irqentry_text_start/end
markers to find out whether a function in a stack trace is
interrupt/exception entry code. This affects the function graph tracer and
filter_irq_stacks().

As the IDT entry points are all sequentialy emitted this is rather simple
to unbreak by injecting __irqentry_text_start/end as global labels.

To make this work correctly:

  - Remove the IRQENTRY_TEXT section from the x86 linker script
  - Define __irqentry so it breaks the build if it's used
  - Adjust the entry mirroring in PTI
  - Remove the redundant kprobes and unwinder bound checks

Reported-by: Qian Cai <cai@lca.pw>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
  • Loading branch information
Thomas Gleixner committed Jun 11, 2020
1 parent 2823e83 commit f0178fc
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 25 deletions.
11 changes: 10 additions & 1 deletion arch/x86/entry/entry_32.S
Original file line number Diff line number Diff line change
Expand Up @@ -743,10 +743,19 @@ SYM_CODE_END(asm_\cfunc)

/*
* Include the defines which emit the idt entries which are shared
* shared between 32 and 64 bit.
* shared between 32 and 64 bit and emit the __irqentry_text_* markers
* so the stacktrace boundary checks work.
*/
.align 16
.globl __irqentry_text_start
__irqentry_text_start:

#include <asm/idtentry.h>

.align 16
.globl __irqentry_text_end
__irqentry_text_end:

/*
* %eax: prev task
* %edx: next task
Expand Down
11 changes: 10 additions & 1 deletion arch/x86/entry/entry_64.S
Original file line number Diff line number Diff line change
Expand Up @@ -478,10 +478,19 @@ SYM_CODE_END(\asmsym)

/*
* Include the defines which emit the idt entries which are shared
* shared between 32 and 64 bit.
* shared between 32 and 64 bit and emit the __irqentry_text_* markers
* so the stacktrace boundary checks work.
*/
.align 16
.globl __irqentry_text_start
__irqentry_text_start:

#include <asm/idtentry.h>

.align 16
.globl __irqentry_text_end
__irqentry_text_end:

SYM_CODE_START_LOCAL(common_interrupt_return)
SYM_INNER_LABEL(swapgs_restore_regs_and_return_to_usermode, SYM_L_GLOBAL)
#ifdef CONFIG_DEBUG_ENTRY
Expand Down
7 changes: 7 additions & 0 deletions arch/x86/include/asm/irq.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
#include <asm/apicdef.h>
#include <asm/irq_vectors.h>

/*
* The irq entry code is in the noinstr section and the start/end of
* __irqentry_text is emitted via labels. Make the build fail if
* something moves a C function into the __irq_entry section.
*/
#define __irq_entry __invalid_section

static inline int irq_canonicalize(int irq)
{
return ((irq == 2) ? 9 : irq);
Expand Down
7 changes: 0 additions & 7 deletions arch/x86/kernel/kprobes/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1073,13 +1073,6 @@ NOKPROBE_SYMBOL(kprobe_fault_handler);

int __init arch_populate_kprobe_blacklist(void)
{
int ret;

ret = kprobe_add_area_blacklist((unsigned long)__irqentry_text_start,
(unsigned long)__irqentry_text_end);
if (ret)
return ret;

return kprobe_add_area_blacklist((unsigned long)__entry_text_start,
(unsigned long)__entry_text_end);
}
Expand Down
4 changes: 1 addition & 3 deletions arch/x86/kernel/kprobes/opt.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,7 @@ static int can_optimize(unsigned long paddr)
* stack handling and registers setup.
*/
if (((paddr >= (unsigned long)__entry_text_start) &&
(paddr < (unsigned long)__entry_text_end)) ||
((paddr >= (unsigned long)__irqentry_text_start) &&
(paddr < (unsigned long)__irqentry_text_end)))
(paddr < (unsigned long)__entry_text_end)))
return 0;

/* Check there is enough space for a relative jump. */
Expand Down
8 changes: 1 addition & 7 deletions arch/x86/kernel/unwind_frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,7 @@ static bool in_entry_code(unsigned long ip)
{
char *addr = (char *)ip;

if (addr >= __entry_text_start && addr < __entry_text_end)
return true;

if (addr >= __irqentry_text_start && addr < __irqentry_text_end)
return true;

return false;
return addr >= __entry_text_start && addr < __entry_text_end;
}

static inline unsigned long *last_frame(struct unwind_state *state)
Expand Down
1 change: 0 additions & 1 deletion arch/x86/kernel/vmlinux.lds.S
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ SECTIONS
KPROBES_TEXT
ALIGN_ENTRY_TEXT_BEGIN
ENTRY_TEXT
IRQENTRY_TEXT
ALIGN_ENTRY_TEXT_END
SOFTIRQENTRY_TEXT
*(.fixup)
Expand Down
4 changes: 2 additions & 2 deletions arch/x86/mm/pti.c
Original file line number Diff line number Diff line change
Expand Up @@ -492,12 +492,12 @@ static void __init pti_setup_espfix64(void)
}

/*
* Clone the populated PMDs of the entry and irqentry text and force it RO.
* Clone the populated PMDs of the entry text and force it RO.
*/
static void pti_clone_entry_text(void)
{
pti_clone_pgtable((unsigned long) __entry_text_start,
(unsigned long) __irqentry_text_end,
(unsigned long) __entry_text_end,
PTI_CLONE_PMD);
}

Expand Down
8 changes: 5 additions & 3 deletions include/linux/interrupt.h
Original file line number Diff line number Diff line change
Expand Up @@ -760,8 +760,10 @@ extern int arch_early_irq_init(void);
/*
* We want to know which function is an entrypoint of a hardirq or a softirq.
*/
#define __irq_entry __attribute__((__section__(".irqentry.text")))
#define __softirq_entry \
__attribute__((__section__(".softirqentry.text")))
#ifndef __irq_entry
# define __irq_entry __attribute__((__section__(".irqentry.text")))
#endif

#define __softirq_entry __attribute__((__section__(".softirqentry.text")))

#endif

0 comments on commit f0178fc

Please sign in to comment.