Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-17 Thread Oleg Nesterov
On 11/16, Thomas Gleixner wrote: > > Subject: x86/dumpstack: Don't try to access user space code of other tasks > From: Thomas Gleixner > Date: Mon, 16 Nov 2020 22:26:52 +0100 > > sysrq-t ends up invoking show_opcodes() for each task which tries to access > the user space code of other processes

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-17 Thread Borislav Petkov
On Mon, Nov 16, 2020 at 11:01:03PM +0100, Thomas Gleixner wrote: > Subject: x86/dumpstack: Don't try to access user space code of other tasks > From: Thomas Gleixner > Date: Mon, 16 Nov 2020 22:26:52 +0100 > > sysrq-t ends up invoking show_opcodes() for each task which tries to access > the user

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-16 Thread Andy Lutomirski
On Mon, Nov 16, 2020 at 3:37 PM Thomas Gleixner wrote: > > On Mon, Nov 16 2020 at 15:04, Andy Lutomirski wrote: > > > On Mon, Nov 16, 2020 at 2:01 PM Thomas Gleixner wrote: > >> arch/x86/kernel/dumpstack.c | 23 +++ > >> 1 file changed, 19 insertions(+), 4 deletions(-) >

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-16 Thread Thomas Gleixner
On Mon, Nov 16 2020 at 15:04, Andy Lutomirski wrote: > On Mon, Nov 16, 2020 at 2:01 PM Thomas Gleixner wrote: >> arch/x86/kernel/dumpstack.c | 23 +++ >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> --- a/arch/x86/kernel/dumpstack.c >> +++

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-16 Thread Andy Lutomirski
On Mon, Nov 16, 2020 at 2:01 PM Thomas Gleixner wrote: > arch/x86/kernel/dumpstack.c | 23 +++ > 1 file changed, 19 insertions(+), 4 deletions(-) > > --- a/arch/x86/kernel/dumpstack.c > +++ b/arch/x86/kernel/dumpstack.c > @@ -78,6 +78,9 @@ static int copy_code(struct

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-16 Thread Thomas Gleixner
On Tue, Nov 03 2020 at 19:20, Borislav Petkov wrote: > On Tue, Nov 03, 2020 at 07:11:15PM +0100, Oleg Nesterov wrote: >> > I'm thinking copy_code() should not use copy_from_user_nmi() if former >> > can be called in non-atomic context too. While copy_from_user_nmi() is named that way, it can be

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-03 Thread Borislav Petkov
On Tue, Nov 03, 2020 at 07:11:15PM +0100, Oleg Nesterov wrote: > > I'm thinking copy_code() should not use copy_from_user_nmi() if former > > can be called in non-atomic context too. > > I understand, but why do you think this makes sense? Because the copy_from_user_nmi()'s name tells me that it

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-03 Thread Oleg Nesterov
On 11/03, Borislav Petkov wrote: > > On Tue, Nov 03, 2020 at 06:47:44PM +0100, Oleg Nesterov wrote: > > > I'm thinking this should not use the atomic variant if it can get called > > > in !atomic context too. > > > > For what? > > I'm thinking copy_code() should not use copy_from_user_nmi() if

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-03 Thread Borislav Petkov
On Tue, Nov 03, 2020 at 06:47:44PM +0100, Oleg Nesterov wrote: > > I'm thinking this should not use the atomic variant if it can get called > > in !atomic context too. > > For what? I'm thinking copy_code() should not use copy_from_user_nmi() if former can be called in non-atomic context too.

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-03 Thread Oleg Nesterov
On 11/03, Borislav Petkov wrote: > > On Tue, Nov 03, 2020 at 01:50:34PM +0100, Oleg Nesterov wrote: > > Another problem is that show_opcodes() makes no sense if user_mode(regs) > > and tsk is not current. > > Because if not current, we would access *some* user address space but > not the one to

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-03 Thread Borislav Petkov
On Tue, Nov 03, 2020 at 01:50:34PM +0100, Oleg Nesterov wrote: > Another problem is that show_opcodes() makes no sense if user_mode(regs) > and tsk is not current. Because if not current, we would access *some* user address space but not the one to which regs belong to? > Try "echo t >

Re: [PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-11-03 Thread Oleg Nesterov
On 10/02, Mark Mossberg wrote: > > Printing "Bad RIP value" if copy_code() fails can be misleading for > userspace pointers, since copy_code() can fail if the instruction > pointer is valid, but the code is paged out. Another problem is that show_opcodes() makes no sense if user_mode(regs) and

[PATCH v2] x86/dumpstack: Fix misleading instruction pointer error message

2020-10-01 Thread Mark Mossberg
Printing "Bad RIP value" if copy_code() fails can be misleading for userspace pointers, since copy_code() can fail if the instruction pointer is valid, but the code is paged out. This is because copy_code() calls copy_from_user_nmi() for userspace pointers, which disables page fault handling.