The normal fixup in handle_bug() is simply continuing at the next
instruction. However upcomming patches make this the wrong thing, so
allow handlers (specifically handle_cfi_failure()) to over-ride
regs->ip.
The callchain is such that the fixup needs to be done before it is
determined if the exception is fatal, as such, revert any changes in
that case.
Additinoally, have handle_cfi_failure() remember the regs->ip value it
starts with for reporting.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/kernel/cfi.c | 8 ++++----
arch/x86/kernel/traps.c | 16 +++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)
--- a/arch/x86/kernel/cfi.c
+++ b/arch/x86/kernel/cfi.c
@@ -67,16 +67,16 @@ static bool decode_cfi_insn(struct pt_re
*/
enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
{
- unsigned long target;
+ unsigned long target, addr = regs->ip;
u32 type;
switch (cfi_mode) {
case CFI_KCFI:
- if (!is_cfi_trap(regs->ip))
+ if (!is_cfi_trap(addr))
return BUG_TRAP_TYPE_NONE;
if (!decode_cfi_insn(regs, &target, &type))
- return report_cfi_failure_noaddr(regs, regs->ip);
+ return report_cfi_failure_noaddr(regs, addr);
break;
@@ -90,7 +90,7 @@ enum bug_trap_type handle_cfi_failure(st
return BUG_TRAP_TYPE_NONE;
}
- return report_cfi_failure(regs, regs->ip, &target, type);
+ return report_cfi_failure(regs, addr, &target, type);
}
/*
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -287,11 +287,12 @@ static inline void handle_invalid_op(str
static noinstr bool handle_bug(struct pt_regs *regs)
{
+ unsigned long addr = regs->ip;
bool handled = false;
int ud_type, ud_len;
s32 ud_imm;
- ud_type = decode_bug(regs->ip, &ud_imm, &ud_len);
+ ud_type = decode_bug(addr, &ud_imm, &ud_len);
if (ud_type == BUG_NONE)
return handled;
@@ -339,8 +340,17 @@ static noinstr bool handle_bug(struct pt
break;
}
- if (handled)
- regs->ip += ud_len;
+ /*
+ * When continuing, and regs->ip hasn't changed, move it to the next
+ * instruction. When not continuing execution, restore the instruction
+ * pointer.
+ */
+ if (handled) {
+ if (regs->ip == addr)
+ regs->ip += ud_len;
+ } else {
+ regs->ip = addr;
+ }
if (regs->flags & X86_EFLAGS_IF)
raw_local_irq_disable();On Mon, Feb 24, 2025 at 01:37:07PM +0100, Peter Zijlstra wrote: > The normal fixup in handle_bug() is simply continuing at the next > instruction. However upcomming patches make this the wrong thing, so > allow handlers (specifically handle_cfi_failure()) to over-ride > regs->ip. > > The callchain is such that the fixup needs to be done before it is > determined if the exception is fatal, as such, revert any changes in > that case. > > Additinoally, have handle_cfi_failure() remember the regs->ip value it typo above, if you want to tweak that in your tree. > starts with for reporting. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Thanks for the added comment, my future brain will appreciate it. :) Reviewed-by: Kees Cook <kees@kernel.org> -- Kees Cook
On Mon, Feb 24, 2025 at 10:59:21AM -0800, Kees Cook wrote: > On Mon, Feb 24, 2025 at 01:37:07PM +0100, Peter Zijlstra wrote: > > The normal fixup in handle_bug() is simply continuing at the next > > instruction. However upcomming patches make this the wrong thing, so > > allow handlers (specifically handle_cfi_failure()) to over-ride > > regs->ip. > > > > The callchain is such that the fixup needs to be done before it is > > determined if the exception is fatal, as such, revert any changes in > > that case. > > > > Additinoally, have handle_cfi_failure() remember the regs->ip value it > > typo above, if you want to tweak that in your tree. Typing is forever hard :-) > > starts with for reporting. > > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Thanks for the added comment, my future brain will appreciate it. :) > > Reviewed-by: Kees Cook <kees@kernel.org> Thanks!
The following commit has been merged into the x86/core branch of tip:
Commit-ID: e33d805a1005bf7b8a5a53559c81a8e17f0b981b
Gitweb: https://git.kernel.org/tip/e33d805a1005bf7b8a5a53559c81a8e17f0b981b
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 24 Feb 2025 13:37:07 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 26 Feb 2025 12:22:39 +01:00
x86/traps: Allow custom fixups in handle_bug()
The normal fixup in handle_bug() is simply continuing at the next
instruction. However upcoming patches make this the wrong thing, so
allow handlers (specifically handle_cfi_failure()) to over-ride
regs->ip.
The callchain is such that the fixup needs to be done before it is
determined if the exception is fatal, as such, revert any changes in
that case.
Additionally, have handle_cfi_failure() remember the regs->ip value it
starts with for reporting.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Kees Cook <kees@kernel.org>
Link: https://lore.kernel.org/r/20250224124200.275223080@infradead.org
---
arch/x86/kernel/cfi.c | 8 ++++----
arch/x86/kernel/traps.c | 16 +++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cfi.c b/arch/x86/kernel/cfi.c
index f6905be..77086cf 100644
--- a/arch/x86/kernel/cfi.c
+++ b/arch/x86/kernel/cfi.c
@@ -67,16 +67,16 @@ static bool decode_cfi_insn(struct pt_regs *regs, unsigned long *target,
*/
enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
{
- unsigned long target;
+ unsigned long target, addr = regs->ip;
u32 type;
switch (cfi_mode) {
case CFI_KCFI:
- if (!is_cfi_trap(regs->ip))
+ if (!is_cfi_trap(addr))
return BUG_TRAP_TYPE_NONE;
if (!decode_cfi_insn(regs, &target, &type))
- return report_cfi_failure_noaddr(regs, regs->ip);
+ return report_cfi_failure_noaddr(regs, addr);
break;
@@ -90,7 +90,7 @@ enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
return BUG_TRAP_TYPE_NONE;
}
- return report_cfi_failure(regs, regs->ip, &target, type);
+ return report_cfi_failure(regs, addr, &target, type);
}
/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index a02a51b..c169f3b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -287,11 +287,12 @@ static inline void handle_invalid_op(struct pt_regs *regs)
static noinstr bool handle_bug(struct pt_regs *regs)
{
+ unsigned long addr = regs->ip;
bool handled = false;
int ud_type, ud_len;
s32 ud_imm;
- ud_type = decode_bug(regs->ip, &ud_imm, &ud_len);
+ ud_type = decode_bug(addr, &ud_imm, &ud_len);
if (ud_type == BUG_NONE)
return handled;
@@ -339,8 +340,17 @@ static noinstr bool handle_bug(struct pt_regs *regs)
break;
}
- if (handled)
- regs->ip += ud_len;
+ /*
+ * When continuing, and regs->ip hasn't changed, move it to the next
+ * instruction. When not continuing execution, restore the instruction
+ * pointer.
+ */
+ if (handled) {
+ if (regs->ip == addr)
+ regs->ip += ud_len;
+ } else {
+ regs->ip = addr;
+ }
if (regs->flags & X86_EFLAGS_IF)
raw_local_irq_disable();The following commit has been merged into the x86/core branch of tip:
Commit-ID: 5d28fddb66e672e3183716a156cae04597599d3b
Gitweb: https://git.kernel.org/tip/5d28fddb66e672e3183716a156cae04597599d3b
Author: Peter Zijlstra <peterz@infradead.org>
AuthorDate: Mon, 24 Feb 2025 13:37:07 +01:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 26 Feb 2025 11:41:54 +01:00
x86/traps: Allow custom fixups in handle_bug()
The normal fixup in handle_bug() is simply continuing at the next
instruction. However upcomming patches make this the wrong thing, so
allow handlers (specifically handle_cfi_failure()) to over-ride
regs->ip.
The callchain is such that the fixup needs to be done before it is
determined if the exception is fatal, as such, revert any changes in
that case.
Additionally, have handle_cfi_failure() remember the regs->ip value it
starts with for reporting.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Kees Cook <kees@kernel.org>
Link: https://lore.kernel.org/r/20250224124200.275223080@infradead.org
---
arch/x86/kernel/cfi.c | 8 ++++----
arch/x86/kernel/traps.c | 16 +++++++++++++---
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cfi.c b/arch/x86/kernel/cfi.c
index f6905be..77086cf 100644
--- a/arch/x86/kernel/cfi.c
+++ b/arch/x86/kernel/cfi.c
@@ -67,16 +67,16 @@ static bool decode_cfi_insn(struct pt_regs *regs, unsigned long *target,
*/
enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
{
- unsigned long target;
+ unsigned long target, addr = regs->ip;
u32 type;
switch (cfi_mode) {
case CFI_KCFI:
- if (!is_cfi_trap(regs->ip))
+ if (!is_cfi_trap(addr))
return BUG_TRAP_TYPE_NONE;
if (!decode_cfi_insn(regs, &target, &type))
- return report_cfi_failure_noaddr(regs, regs->ip);
+ return report_cfi_failure_noaddr(regs, addr);
break;
@@ -90,7 +90,7 @@ enum bug_trap_type handle_cfi_failure(struct pt_regs *regs)
return BUG_TRAP_TYPE_NONE;
}
- return report_cfi_failure(regs, regs->ip, &target, type);
+ return report_cfi_failure(regs, addr, &target, type);
}
/*
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index a02a51b..c169f3b 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -287,11 +287,12 @@ static inline void handle_invalid_op(struct pt_regs *regs)
static noinstr bool handle_bug(struct pt_regs *regs)
{
+ unsigned long addr = regs->ip;
bool handled = false;
int ud_type, ud_len;
s32 ud_imm;
- ud_type = decode_bug(regs->ip, &ud_imm, &ud_len);
+ ud_type = decode_bug(addr, &ud_imm, &ud_len);
if (ud_type == BUG_NONE)
return handled;
@@ -339,8 +340,17 @@ static noinstr bool handle_bug(struct pt_regs *regs)
break;
}
- if (handled)
- regs->ip += ud_len;
+ /*
+ * When continuing, and regs->ip hasn't changed, move it to the next
+ * instruction. When not continuing execution, restore the instruction
+ * pointer.
+ */
+ if (handled) {
+ if (regs->ip == addr)
+ regs->ip += ud_len;
+ } else {
+ regs->ip = addr;
+ }
if (regs->flags & X86_EFLAGS_IF)
raw_local_irq_disable();© 2016 - 2025 Red Hat, Inc.