v4:
-- Add BUG() at the end of nmi_exception_handler()
-- Return earlier in die() if notify_die() returns NOTIFY_STOP
-- Update the patch titles and commit messages
v3:
-- Make each patch can be built without errors and warnings.
v2:
-- Update the commit message to give more detailed info, split into
three individual patches, suggested by Maciej, thank you.
Tiezhu Yang (4):
MIPS: Add BUG() at the end of nmi_exception_handler()
MIPS: Do not kill the task in die() if notify_die() returns
NOTIFY_STOP
MIPS: Return earlier in die() if notify_die() returns NOTIFY_STOP
MIPS: Add identifier names to arguments of die() declaration
arch/mips/include/asm/ptrace.h | 2 +-
arch/mips/kernel/traps.c | 15 +++++++++------
2 files changed, 10 insertions(+), 7 deletions(-)
--
2.1.0
If notify_die() returns NOTIFY_STOP, honor the return value from the
handler chain invocation in die() and return without killing the task
as, through a debugger, the fault may have been fixed. It makes sense
even if ignoring the event will make the system unstable: by allowing
access through a debugger it has been compromised already anyway. It
makes our port consistent with x86, arm64, riscv and csky.
Commit 20c0d2d44029 ("[PATCH] i386: pass proper trap numbers to die
chain handlers") may be the earliest of similar changes.
Link: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Tiezhu Yang <[email protected]>
---
arch/mips/include/asm/ptrace.h | 2 +-
arch/mips/kernel/traps.c | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
index daf3cf2..aee8e0a 100644
--- a/arch/mips/include/asm/ptrace.h
+++ b/arch/mips/include/asm/ptrace.h
@@ -159,7 +159,7 @@ static inline long regs_return_value(struct pt_regs *regs)
extern asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall);
extern asmlinkage void syscall_trace_leave(struct pt_regs *regs);
-extern void die(const char *, struct pt_regs *) __noreturn;
+extern void die(const char *, struct pt_regs *);
static inline void die_if_kernel(const char *str, struct pt_regs *regs)
{
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index b1b68ce..8e528a8 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -391,16 +391,15 @@ void show_registers(struct pt_regs *regs)
static DEFINE_RAW_SPINLOCK(die_lock);
-void __noreturn die(const char *str, struct pt_regs *regs)
+void die(const char *str, struct pt_regs *regs)
{
static int die_counter;
- int sig = SIGSEGV;
+ int ret;
oops_enter();
- if (notify_die(DIE_OOPS, str, regs, 0, current->thread.trap_nr,
- SIGSEGV) == NOTIFY_STOP)
- sig = 0;
+ ret = notify_die(DIE_OOPS, str, regs, 0,
+ current->thread.trap_nr, SIGSEGV);
console_verbose();
raw_spin_lock_irq(&die_lock);
@@ -422,7 +421,8 @@ void __noreturn die(const char *str, struct pt_regs *regs)
if (regs && kexec_should_crash(current))
crash_kexec(regs);
- make_task_dead(sig);
+ if (ret != NOTIFY_STOP)
+ make_task_dead(SIGSEGV);
}
extern struct exception_table_entry __start___dbe_table[];
--
2.1.0
On Sat, Aug 19, 2023 at 04:36:19PM +0800, Tiezhu Yang wrote:
> v4:
> -- Add BUG() at the end of nmi_exception_handler()
> -- Return earlier in die() if notify_die() returns NOTIFY_STOP
> -- Update the patch titles and commit messages
>
> v3:
> -- Make each patch can be built without errors and warnings.
>
> v2:
> -- Update the commit message to give more detailed info, split into
> three individual patches, suggested by Maciej, thank you.
>
> Tiezhu Yang (4):
> MIPS: Add BUG() at the end of nmi_exception_handler()
> MIPS: Do not kill the task in die() if notify_die() returns
> NOTIFY_STOP
> MIPS: Return earlier in die() if notify_die() returns NOTIFY_STOP
> MIPS: Add identifier names to arguments of die() declaration
>
> arch/mips/include/asm/ptrace.h | 2 +-
> arch/mips/kernel/traps.c | 15 +++++++++------
> 2 files changed, 10 insertions(+), 7 deletions(-)
series applied to mips-next.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
On Mon, Aug 28, 2023 at 11:11:19AM +0200, Thomas Bogendoerfer wrote:
> On Sat, Aug 19, 2023 at 04:36:19PM +0800, Tiezhu Yang wrote:
> > v4:
> > -- Add BUG() at the end of nmi_exception_handler()
> > -- Return earlier in die() if notify_die() returns NOTIFY_STOP
> > -- Update the patch titles and commit messages
> >
> > v3:
> > -- Make each patch can be built without errors and warnings.
> >
> > v2:
> > -- Update the commit message to give more detailed info, split into
> > three individual patches, suggested by Maciej, thank you.
> >
> > Tiezhu Yang (4):
> > MIPS: Add BUG() at the end of nmi_exception_handler()
> > MIPS: Do not kill the task in die() if notify_die() returns
> > NOTIFY_STOP
> > MIPS: Return earlier in die() if notify_die() returns NOTIFY_STOP
> > MIPS: Add identifier names to arguments of die() declaration
> >
> > arch/mips/include/asm/ptrace.h | 2 +-
> > arch/mips/kernel/traps.c | 15 +++++++++------
> > 2 files changed, 10 insertions(+), 7 deletions(-)
>
> series applied to mips-next.
I've dropped the series again after feedback from Maciej, that this
still needs more changes.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
Hi, Thomas,
On Tue, Aug 29, 2023 at 11:44 PM Thomas Bogendoerfer
<[email protected]> wrote:
>
> On Mon, Aug 28, 2023 at 11:11:19AM +0200, Thomas Bogendoerfer wrote:
> > On Sat, Aug 19, 2023 at 04:36:19PM +0800, Tiezhu Yang wrote:
> > > v4:
> > > -- Add BUG() at the end of nmi_exception_handler()
> > > -- Return earlier in die() if notify_die() returns NOTIFY_STOP
> > > -- Update the patch titles and commit messages
> > >
> > > v3:
> > > -- Make each patch can be built without errors and warnings.
> > >
> > > v2:
> > > -- Update the commit message to give more detailed info, split into
> > > three individual patches, suggested by Maciej, thank you.
> > >
> > > Tiezhu Yang (4):
> > > MIPS: Add BUG() at the end of nmi_exception_handler()
> > > MIPS: Do not kill the task in die() if notify_die() returns
> > > NOTIFY_STOP
> > > MIPS: Return earlier in die() if notify_die() returns NOTIFY_STOP
> > > MIPS: Add identifier names to arguments of die() declaration
> > >
> > > arch/mips/include/asm/ptrace.h | 2 +-
> > > arch/mips/kernel/traps.c | 15 +++++++++------
> > > 2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > series applied to mips-next.
>
> I've dropped the series again after feedback from Maciej, that this
> still needs more changes.
I feel a little surprised. This series has appeared for more than ten
days and received some R-b, and we haven't seen any objections from
Maciej. If there are really some bugs that need to be fixed, I think
the normal operation is making additional patches...
Huacai
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]
在 2023/8/29 23:19, Thomas Bogendoerfer 写道:
> On Mon, Aug 28, 2023 at 11:11:19AM +0200, Thomas Bogendoerfer wrote:
>> On Sat, Aug 19, 2023 at 04:36:19PM +0800, Tiezhu Yang wrote:
>>> v4:
>>> -- Add BUG() at the end of nmi_exception_handler()
>>> -- Return earlier in die() if notify_die() returns NOTIFY_STOP
>>> -- Update the patch titles and commit messages
>>>
>>> v3:
>>> -- Make each patch can be built without errors and warnings.
>>>
>>> v2:
>>> -- Update the commit message to give more detailed info, split into
>>> three individual patches, suggested by Maciej, thank you.
>>>
>>> Tiezhu Yang (4):
>>> MIPS: Add BUG() at the end of nmi_exception_handler()
>>> MIPS: Do not kill the task in die() if notify_die() returns
>>> NOTIFY_STOP
>>> MIPS: Return earlier in die() if notify_die() returns NOTIFY_STOP
>>> MIPS: Add identifier names to arguments of die() declaration
>>>
>>> arch/mips/include/asm/ptrace.h | 2 +-
>>> arch/mips/kernel/traps.c | 15 +++++++++------
>>> 2 files changed, 10 insertions(+), 7 deletions(-)
>> series applied to mips-next.
> I've dropped the series again after feedback from Maciej, that this
> still needs more changes.
Was the feedback made off-list?
I think it's important to make submitter aware of the problem before
action being taken.
From my perspective Teizhu was working hard to address review comments
all the way so it looks unfair to neglect his work without transparent
communication.
Thanks
- Jiaxun
> Thomas.