* [PATCH v2] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call @ 2018-11-16 11:17 Elvira Khabirova 2018-11-16 12:42 ` Michael Ellerman 0 siblings, 1 reply; 20+ messages in thread From: Elvira Khabirova @ 2018-11-16 11:17 UTC (permalink / raw) To: benh, paulus, mpe Cc: esyr, linux-kernel, ldv, luto, oleg, leitao, linuxppc-dev Arch code should use tracehook_*() helpers, as documented in include/linux/tracehook.h. Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> --- arch/powerpc/kernel/ptrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index afb819f4ca68..d79d907f9acc 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3266,7 +3266,7 @@ long do_syscall_trace_enter(struct pt_regs *regs) user_exit(); if (test_thread_flag(TIF_SYSCALL_EMU)) { - ptrace_report_syscall(regs); + (void) tracehook_report_syscall_entry(regs); /* * Returning -1 will skip the syscall execution. We want to * avoid clobbering any register also, thus, not 'gotoing' -- 2.19.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-11-16 11:17 [PATCH v2] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call Elvira Khabirova @ 2018-11-16 12:42 ` Michael Ellerman 2018-11-19 21:01 ` [PATCH v3] " Dmitry V. Levin 0 siblings, 1 reply; 20+ messages in thread From: Michael Ellerman @ 2018-11-16 12:42 UTC (permalink / raw) To: Elvira Khabirova, benh, paulus Cc: esyr, linux-kernel, ldv, luto, oleg, leitao, linuxppc-dev Elvira Khabirova <lineprinter@altlinux.org> writes: > Arch code should use tracehook_*() helpers, as documented > in include/linux/tracehook.h. Thanks. It probably also should have a comment explaining why we're ignoring the return value and why that's OK. cheers > Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") > Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > --- > arch/powerpc/kernel/ptrace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index afb819f4ca68..d79d907f9acc 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -3266,7 +3266,7 @@ long do_syscall_trace_enter(struct pt_regs *regs) > user_exit(); > > if (test_thread_flag(TIF_SYSCALL_EMU)) { > - ptrace_report_syscall(regs); > + (void) tracehook_report_syscall_entry(regs); > /* > * Returning -1 will skip the syscall execution. We want to > * avoid clobbering any register also, thus, not 'gotoing' > -- > 2.19.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-11-16 12:42 ` Michael Ellerman @ 2018-11-19 21:01 ` Dmitry V. Levin 2018-11-21 21:17 ` Michael Ellerman 0 siblings, 1 reply; 20+ messages in thread From: Dmitry V. Levin @ 2018-11-19 21:01 UTC (permalink / raw) To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras Cc: linux-kernel, Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Andy Lutomirski, Breno Leitao, linuxppc-dev From: Elvira Khabirova <lineprinter@altlinux.org> Arch code should use tracehook_*() helpers as documented in include/linux/tracehook.h, ptrace_report_syscall() is not expected to be used outside that file. Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- v3: add a descriptive comment v2: explicitly ignore tracehook_report_syscall_entry() return code arch/powerpc/kernel/ptrace.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index afb819f4ca68..e84220d91bbd 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3266,7 +3266,12 @@ long do_syscall_trace_enter(struct pt_regs *regs) user_exit(); if (test_thread_flag(TIF_SYSCALL_EMU)) { - ptrace_report_syscall(regs); + /* + * A nonzero return code from tracehook_report_syscall_entry() + * tells us to prevent the syscall execution, but we are not + * going to execute it anyway. + */ + (void) tracehook_report_syscall_entry(regs); /* * Returning -1 will skip the syscall execution. We want to * avoid clobbering any register also, thus, not 'gotoing' -- ldv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-11-19 21:01 ` [PATCH v3] " Dmitry V. Levin @ 2018-11-21 21:17 ` Michael Ellerman 2018-12-03 3:18 ` [PATCH v4] " Dmitry V. Levin 0 siblings, 1 reply; 20+ messages in thread From: Michael Ellerman @ 2018-11-21 21:17 UTC (permalink / raw) To: Dmitry V. Levin, Benjamin Herrenschmidt, Paul Mackerras Cc: linux-kernel, Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Andy Lutomirski, Breno Leitao, linuxppc-dev Hi Dmitry, Thanks for the patch. "Dmitry V. Levin" <ldv@altlinux.org> writes: > From: Elvira Khabirova <lineprinter@altlinux.org> > > Arch code should use tracehook_*() helpers as documented > in include/linux/tracehook.h, > ptrace_report_syscall() is not expected to be used outside that file. > > Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> > Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") > Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > --- > > v3: add a descriptive comment > v2: explicitly ignore tracehook_report_syscall_entry() return code > > arch/powerpc/kernel/ptrace.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index afb819f4ca68..e84220d91bbd 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -3266,7 +3266,12 @@ long do_syscall_trace_enter(struct pt_regs *regs) > user_exit(); > > if (test_thread_flag(TIF_SYSCALL_EMU)) { > - ptrace_report_syscall(regs); > + /* > + * A nonzero return code from tracehook_report_syscall_entry() > + * tells us to prevent the syscall execution, but we are not > + * going to execute it anyway. > + */ > + (void) tracehook_report_syscall_entry(regs); Unfortunately the (void) cast doesn't work to suppress the must check warning. arch/powerpc/kernel/ptrace.c:3274:3: error: ignoring return value of 'tracehook_report_syscall_entry', declared with attribute warn_unused_result [-Werror=unused-result] AFAIK we don't have a way to suppress that. I guess we should rewrite it to only call tracehook_report_syscall_entry() once, like x86 does. I'll try and get a patch for that done & tested. cheers ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-11-21 21:17 ` Michael Ellerman @ 2018-12-03 3:18 ` Dmitry V. Levin 2018-12-07 1:19 ` Dmitry V. Levin 0 siblings, 1 reply; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-03 3:18 UTC (permalink / raw) To: Michael Ellerman Cc: Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev, linux-kernel From: Elvira Khabirova <lineprinter@altlinux.org> Arch code should use tracehook_*() helpers, as documented in include/linux/tracehook.h, ptrace_report_syscall() is not expected to be used outside that file. Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested v3: add a descriptive comment v2: explicitly ignore tracehook_report_syscall_entry() return code arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index afb819f4ca68..59c8c9a3d7ea 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3263,27 +3263,43 @@ static inline int do_seccomp(struct pt_regs *regs) { return 0; } */ long do_syscall_trace_enter(struct pt_regs *regs) { + struct thread_info *ti; + u32 cached_flags; + user_exit(); - if (test_thread_flag(TIF_SYSCALL_EMU)) { - ptrace_report_syscall(regs); - /* - * Returning -1 will skip the syscall execution. We want to - * avoid clobbering any register also, thus, not 'gotoing' - * skip label. - */ - return -1; - } + ti = current_thread_info(); + cached_flags = + READ_ONCE(ti->flags) & + (TIF_SYSCALL_EMU | TIF_SYSCALL_TRACE | TIF_SYSCALL_TRACEPOINT); - /* - * The tracer may decide to abort the syscall, if so tracehook - * will return !0. Note that the tracer may also just change - * regs->gpr[0] to an invalid syscall number, that is handled - * below on the exit path. - */ - if (test_thread_flag(TIF_SYSCALL_TRACE) && - tracehook_report_syscall_entry(regs)) - goto skip; + if (cached_flags & (TIF_SYSCALL_EMU | TIF_SYSCALL_TRACE)) { + int rc = tracehook_report_syscall_entry(regs); + + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { + /* + * A nonzero return code from + * tracehook_report_syscall_entry() tells us + * to prevent the syscall execution, but + * we are not going to execute it anyway. + * + * Returning -1 will skip the syscall execution. + * We want to avoid clobbering any register also, + * thus, not 'gotoing' skip label. + */ + return -1; + } + + if (rc) { + /* + * The tracer decided to abort the syscall. + * Note that the tracer may also just change + * regs->gpr[0] to an invalid syscall number, + * that is handled below on the exit path. + */ + goto skip; + } + } /* Run seccomp after ptrace; allow it to set gpr[3]. */ if (do_seccomp(regs)) @@ -3293,7 +3309,7 @@ long do_syscall_trace_enter(struct pt_regs *regs) if (regs->gpr[0] >= NR_syscalls) goto skip; - if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) + if (unlikely(cached_flags & TIF_SYSCALL_TRACEPOINT)) trace_sys_enter(regs, regs->gpr[0]); #ifdef CONFIG_PPC64 -- ldv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-03 3:18 ` [PATCH v4] " Dmitry V. Levin @ 2018-12-07 1:19 ` Dmitry V. Levin 2018-12-07 11:12 ` Michael Ellerman 0 siblings, 1 reply; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-07 1:19 UTC (permalink / raw) To: Michael Ellerman Cc: Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1129 bytes --] On Mon, Dec 03, 2018 at 06:18:23AM +0300, Dmitry V. Levin wrote: > From: Elvira Khabirova <lineprinter@altlinux.org> > > Arch code should use tracehook_*() helpers, as documented > in include/linux/tracehook.h, > ptrace_report_syscall() is not expected to be used outside that file. > > Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> > Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") > Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > --- > v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested > v3: add a descriptive comment > v2: explicitly ignore tracehook_report_syscall_entry() return code > > arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++++------------- > 1 file changed, 35 insertions(+), 19 deletions(-) Sorry, this patch does not work, please ignore it. However, the bug blocks PTRACE_GET_SYSCALL_INFO, so please fix it. I'm going to use if (tracehook_report_syscall_entry(regs)) return -1; return -1; in the series until you have a better fix. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 1:19 ` Dmitry V. Levin @ 2018-12-07 11:12 ` Michael Ellerman 2018-12-07 15:42 ` Dmitry V. Levin 0 siblings, 1 reply; 20+ messages in thread From: Michael Ellerman @ 2018-12-07 11:12 UTC (permalink / raw) To: Dmitry V. Levin Cc: Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev, linux-kernel "Dmitry V. Levin" <ldv@altlinux.org> writes: > On Mon, Dec 03, 2018 at 06:18:23AM +0300, Dmitry V. Levin wrote: >> From: Elvira Khabirova <lineprinter@altlinux.org> >> >> Arch code should use tracehook_*() helpers, as documented >> in include/linux/tracehook.h, >> ptrace_report_syscall() is not expected to be used outside that file. >> >> Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> >> Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") >> Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> >> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> >> --- >> v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested >> v3: add a descriptive comment >> v2: explicitly ignore tracehook_report_syscall_entry() return code >> >> arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++++------------- >> 1 file changed, 35 insertions(+), 19 deletions(-) > > Sorry, this patch does not work, please ignore it. Hmm OK. Why exactly? I wrote more or less the same patch, although I used a temporary bool. > However, the bug blocks PTRACE_GET_SYSCALL_INFO, so please fix it. Sorry, didn't realise it was blocking you. > I'm going to use > if (tracehook_report_syscall_entry(regs)) > return -1; > return -1; > in the series until you have a better fix. Yeah that's fine by me. I could send that to Linus for 4.20 if you want me to, otherwise I'm fine for you to carry it in your series. cheers ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 11:12 ` Michael Ellerman @ 2018-12-07 15:42 ` Dmitry V. Levin 2018-12-07 15:56 ` [PATCH v5] " Dmitry V. Levin 2018-12-07 16:34 ` [PATCH v4] " Oleg Nesterov 0 siblings, 2 replies; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-07 15:42 UTC (permalink / raw) To: Michael Ellerman Cc: Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2035 bytes --] On Fri, Dec 07, 2018 at 10:12:49PM +1100, Michael Ellerman wrote: > "Dmitry V. Levin" <ldv@altlinux.org> writes: > > On Mon, Dec 03, 2018 at 06:18:23AM +0300, Dmitry V. Levin wrote: > >> From: Elvira Khabirova <lineprinter@altlinux.org> > >> > >> Arch code should use tracehook_*() helpers, as documented > >> in include/linux/tracehook.h, > >> ptrace_report_syscall() is not expected to be used outside that file. > >> > >> Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> > >> Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") > >> Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > >> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > >> --- > >> v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested > >> v3: add a descriptive comment > >> v2: explicitly ignore tracehook_report_syscall_entry() return code > >> > >> arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++++------------- > >> 1 file changed, 35 insertions(+), 19 deletions(-) > > > > Sorry, this patch does not work, please ignore it. > > Hmm OK. Why exactly? Unfortunately, I have no idea why it doesn't work. All I can say is it breaks strace because the kernel no longer sends syscall entry stops. > I wrote more or less the same patch, although I used a temporary bool. > > > However, the bug blocks PTRACE_GET_SYSCALL_INFO, so please fix it. > > Sorry, didn't realise it was blocking you. We are changing ptrace_report_syscall signature to implement PTRACE_GET_SYSCALL_INFO, and this is the only place in the kernel besides tracehook_report_syscall_*() that invokes ptrace_report_syscall() directly. > > I'm going to use > > if (tracehook_report_syscall_entry(regs)) > > return -1; > > return -1; > > in the series until you have a better fix. > > Yeah that's fine by me. I could send that to Linus for 4.20 if you want > me to, otherwise I'm fine for you to carry it in your series. Yes, please. I'll send a v5 shortly. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 15:42 ` Dmitry V. Levin @ 2018-12-07 15:56 ` Dmitry V. Levin 2018-12-07 18:52 ` [PATCH v6] " Dmitry V. Levin 2018-12-11 13:45 ` [v5] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call Michael Ellerman 2018-12-07 16:34 ` [PATCH v4] " Oleg Nesterov 1 sibling, 2 replies; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-07 15:56 UTC (permalink / raw) To: Michael Ellerman Cc: Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev, linux-kernel From: Elvira Khabirova <lineprinter@altlinux.org> Arch code should use tracehook_*() helpers, as documented in include/linux/tracehook.h, ptrace_report_syscall() is not expected to be used outside that file. The patch does not look very nice, but at least it is correct and opens the way for PTRACE_GET_SYSCALL_INFO API. Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Breno Leitao <leitao@debian.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Eugene Syromyatnikov <esyr@redhat.com> Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- v5: reverted to a simple approach, compile- and run-tested v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested v3: add a descriptive comment v2: explicitly ignore tracehook_report_syscall_entry() return code arch/powerpc/kernel/ptrace.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index afb819f4ca68..714c3480c52d 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3266,12 +3266,17 @@ long do_syscall_trace_enter(struct pt_regs *regs) user_exit(); if (test_thread_flag(TIF_SYSCALL_EMU)) { - ptrace_report_syscall(regs); /* + * A nonzero return code from tracehook_report_syscall_entry() + * tells us to prevent the syscall execution, but we are not + * going to execute it anyway. + * * Returning -1 will skip the syscall execution. We want to * avoid clobbering any register also, thus, not 'gotoing' * skip label. */ + if (tracehook_report_syscall_entry(regs)) + ; return -1; } -- ldv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v6] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 15:56 ` [PATCH v5] " Dmitry V. Levin @ 2018-12-07 18:52 ` Dmitry V. Levin 2018-12-10 13:28 ` Oleg Nesterov 2018-12-11 13:45 ` [v5] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call Michael Ellerman 1 sibling, 1 reply; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-07 18:52 UTC (permalink / raw) To: Michael Ellerman Cc: Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev, linux-kernel From: Elvira Khabirova <lineprinter@altlinux.org> Arch code should use tracehook_*() helpers, as documented in include/linux/tracehook.h, ptrace_report_syscall() is not expected to be used outside that file. Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Breno Leitao <leitao@debian.org> Cc: Andy Lutomirski <luto@kernel.org> Cc: Eugene Syromyatnikov <esyr@redhat.com> Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- Please make either v5 or v6 edition of this fix, or any similar fix, into v4.20. v6: reverted to a fixed version of v4, compile- and run-tested with strace v5: reverted to a simple approach, compile- and run-tested v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested v3: add a descriptive comment v2: explicitly ignore tracehook_report_syscall_entry() return code arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index afb819f4ca68..fcfdc1229f08 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3263,27 +3263,43 @@ static inline int do_seccomp(struct pt_regs *regs) { return 0; } */ long do_syscall_trace_enter(struct pt_regs *regs) { + struct thread_info *ti; + u32 cached_flags; + user_exit(); - if (test_thread_flag(TIF_SYSCALL_EMU)) { - ptrace_report_syscall(regs); - /* - * Returning -1 will skip the syscall execution. We want to - * avoid clobbering any register also, thus, not 'gotoing' - * skip label. - */ - return -1; - } + ti = current_thread_info(); + cached_flags = READ_ONCE(ti->flags) & + (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE | + _TIF_SYSCALL_TRACEPOINT); - /* - * The tracer may decide to abort the syscall, if so tracehook - * will return !0. Note that the tracer may also just change - * regs->gpr[0] to an invalid syscall number, that is handled - * below on the exit path. - */ - if (test_thread_flag(TIF_SYSCALL_TRACE) && - tracehook_report_syscall_entry(regs)) - goto skip; + if (cached_flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { + int rc = tracehook_report_syscall_entry(regs); + + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { + /* + * A nonzero return code from + * tracehook_report_syscall_entry() tells us + * to prevent the syscall execution, but + * we are not going to execute it anyway. + * + * Returning -1 will skip the syscall execution. + * We want to avoid clobbering any register also, + * thus, not 'gotoing' skip label. + */ + return -1; + } + + if (rc) { + /* + * The tracer decided to abort the syscall. + * Note that the tracer may also just change + * regs->gpr[0] to an invalid syscall number, + * that is handled below on the exit path. + */ + goto skip; + } + } /* Run seccomp after ptrace; allow it to set gpr[3]. */ if (do_seccomp(regs)) @@ -3293,7 +3309,7 @@ long do_syscall_trace_enter(struct pt_regs *regs) if (regs->gpr[0] >= NR_syscalls) goto skip; - if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) + if (unlikely(cached_flags & _TIF_SYSCALL_TRACEPOINT)) trace_sys_enter(regs, regs->gpr[0]); #ifdef CONFIG_PPC64 -- ldv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v6] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 18:52 ` [PATCH v6] " Dmitry V. Levin @ 2018-12-10 13:28 ` Oleg Nesterov 2018-12-10 13:36 ` Dmitry V. Levin 0 siblings, 1 reply; 20+ messages in thread From: Oleg Nesterov @ 2018-12-10 13:28 UTC (permalink / raw) To: Dmitry V. Levin Cc: Eugene Syromyatnikov, linux-kernel, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev On 12/07, Dmitry V. Levin wrote: > > Please make either v5 or v6 edition of this fix, or any similar fix, > into v4.20. IIUC, v5 above means [PATCH v5 23/25] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call you sent in another series... > long do_syscall_trace_enter(struct pt_regs *regs) > { > + struct thread_info *ti; > + u32 cached_flags; > + > user_exit(); > > - if (test_thread_flag(TIF_SYSCALL_EMU)) { > - ptrace_report_syscall(regs); > - /* > - * Returning -1 will skip the syscall execution. We want to > - * avoid clobbering any register also, thus, not 'gotoing' > - * skip label. > - */ > - return -1; > - } > + ti = current_thread_info(); > + cached_flags = READ_ONCE(ti->flags) & > + (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE | > + _TIF_SYSCALL_TRACEPOINT); > > - /* > - * The tracer may decide to abort the syscall, if so tracehook > - * will return !0. Note that the tracer may also just change > - * regs->gpr[0] to an invalid syscall number, that is handled > - * below on the exit path. > - */ > - if (test_thread_flag(TIF_SYSCALL_TRACE) && > - tracehook_report_syscall_entry(regs)) > - goto skip; > + if (cached_flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { > + int rc = tracehook_report_syscall_entry(regs); > + > + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { > + /* > + * A nonzero return code from > + * tracehook_report_syscall_entry() tells us > + * to prevent the syscall execution, but > + * we are not going to execute it anyway. > + * > + * Returning -1 will skip the syscall execution. > + * We want to avoid clobbering any register also, > + * thus, not 'gotoing' skip label. > + */ > + return -1; > + } > + > + if (rc) { > + /* > + * The tracer decided to abort the syscall. > + * Note that the tracer may also just change > + * regs->gpr[0] to an invalid syscall number, > + * that is handled below on the exit path. > + */ > + goto skip; > + } > + } > > /* Run seccomp after ptrace; allow it to set gpr[3]. */ > if (do_seccomp(regs)) > @@ -3293,7 +3309,7 @@ long do_syscall_trace_enter(struct pt_regs *regs) > if (regs->gpr[0] >= NR_syscalls) > goto skip; > > - if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > + if (unlikely(cached_flags & _TIF_SYSCALL_TRACEPOINT)) I will leave this to maintainers, but to me this change looks good and imo it also cleanups the code. However I am not sure cached_flags should include _TIF_SYSCALL_TRACEPOINT. If nothing else, the caller can sleep in ptrace_stop() unpredictably long and TIF_SYSCALL_TRACEPOINT can be set/cleared meanwhile. Oleg. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v6] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-10 13:28 ` Oleg Nesterov @ 2018-12-10 13:36 ` Dmitry V. Levin 2018-12-16 17:28 ` [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter Dmitry V. Levin 0 siblings, 1 reply; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-10 13:36 UTC (permalink / raw) To: Oleg Nesterov Cc: Eugene Syromyatnikov, linux-kernel, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 3167 bytes --] On Mon, Dec 10, 2018 at 02:28:07PM +0100, Oleg Nesterov wrote: > On 12/07, Dmitry V. Levin wrote: > > > > Please make either v5 or v6 edition of this fix, or any similar fix, > > into v4.20. > > IIUC, v5 above means > > [PATCH v5 23/25] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call > > you sent in another series... They just happen to have the same v5 here and there. In that series I included the most trivial variant of the change. > > long do_syscall_trace_enter(struct pt_regs *regs) > > { > > + struct thread_info *ti; > > + u32 cached_flags; > > + > > user_exit(); > > > > - if (test_thread_flag(TIF_SYSCALL_EMU)) { > > - ptrace_report_syscall(regs); > > - /* > > - * Returning -1 will skip the syscall execution. We want to > > - * avoid clobbering any register also, thus, not 'gotoing' > > - * skip label. > > - */ > > - return -1; > > - } > > + ti = current_thread_info(); > > + cached_flags = READ_ONCE(ti->flags) & > > + (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE | > > + _TIF_SYSCALL_TRACEPOINT); > > > > - /* > > - * The tracer may decide to abort the syscall, if so tracehook > > - * will return !0. Note that the tracer may also just change > > - * regs->gpr[0] to an invalid syscall number, that is handled > > - * below on the exit path. > > - */ > > - if (test_thread_flag(TIF_SYSCALL_TRACE) && > > - tracehook_report_syscall_entry(regs)) > > - goto skip; > > + if (cached_flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { > > + int rc = tracehook_report_syscall_entry(regs); > > + > > + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { > > + /* > > + * A nonzero return code from > > + * tracehook_report_syscall_entry() tells us > > + * to prevent the syscall execution, but > > + * we are not going to execute it anyway. > > + * > > + * Returning -1 will skip the syscall execution. > > + * We want to avoid clobbering any register also, > > + * thus, not 'gotoing' skip label. > > + */ > > + return -1; > > + } > > + > > + if (rc) { > > + /* > > + * The tracer decided to abort the syscall. > > + * Note that the tracer may also just change > > + * regs->gpr[0] to an invalid syscall number, > > + * that is handled below on the exit path. > > + */ > > + goto skip; > > + } > > + } > > > > /* Run seccomp after ptrace; allow it to set gpr[3]. */ > > if (do_seccomp(regs)) > > @@ -3293,7 +3309,7 @@ long do_syscall_trace_enter(struct pt_regs *regs) > > if (regs->gpr[0] >= NR_syscalls) > > goto skip; > > > > - if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > > + if (unlikely(cached_flags & _TIF_SYSCALL_TRACEPOINT)) > > I will leave this to maintainers, but to me this change looks good and imo it > also cleanups the code. > > However I am not sure cached_flags should include _TIF_SYSCALL_TRACEPOINT. If > nothing else, the caller can sleep in ptrace_stop() unpredictably long and > TIF_SYSCALL_TRACEPOINT can be set/cleared meanwhile. I agree, we shouldn't cache _TIF_SYSCALL_TRACEPOINT. -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter 2018-12-10 13:36 ` Dmitry V. Levin @ 2018-12-16 17:28 ` Dmitry V. Levin 2018-12-17 11:20 ` Michael Ellerman ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-16 17:28 UTC (permalink / raw) To: Michael Ellerman, Oleg Nesterov; +Cc: linuxppc-dev, linux-kernel Invoke tracehook_report_syscall_entry once. Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> --- arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 714c3480c52d..8794d32c2d9e 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -3263,32 +3263,40 @@ static inline int do_seccomp(struct pt_regs *regs) { return 0; } */ long do_syscall_trace_enter(struct pt_regs *regs) { + u32 cached_flags; + user_exit(); - if (test_thread_flag(TIF_SYSCALL_EMU)) { - /* - * A nonzero return code from tracehook_report_syscall_entry() - * tells us to prevent the syscall execution, but we are not - * going to execute it anyway. - * - * Returning -1 will skip the syscall execution. We want to - * avoid clobbering any register also, thus, not 'gotoing' - * skip label. - */ - if (tracehook_report_syscall_entry(regs)) - ; - return -1; - } + cached_flags = READ_ONCE(current_thread_info()->flags) & + (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE); - /* - * The tracer may decide to abort the syscall, if so tracehook - * will return !0. Note that the tracer may also just change - * regs->gpr[0] to an invalid syscall number, that is handled - * below on the exit path. - */ - if (test_thread_flag(TIF_SYSCALL_TRACE) && - tracehook_report_syscall_entry(regs)) - goto skip; + if (cached_flags) { + int rc = tracehook_report_syscall_entry(regs); + + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { + /* + * A nonzero return code from + * tracehook_report_syscall_entry() tells us + * to prevent the syscall execution, but + * we are not going to execute it anyway. + * + * Returning -1 will skip the syscall execution. + * We want to avoid clobbering any register also, + * thus, not 'gotoing' skip label. + */ + return -1; + } + + if (rc) { + /* + * The tracer decided to abort the syscall. + * Note that the tracer may also just change + * regs->gpr[0] to an invalid syscall number, + * that is handled below on the exit path. + */ + goto skip; + } + } /* Run seccomp after ptrace; allow it to set gpr[3]. */ if (do_seccomp(regs)) -- ldv ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter 2018-12-16 17:28 ` [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter Dmitry V. Levin @ 2018-12-17 11:20 ` Michael Ellerman 2018-12-17 11:23 ` Dmitry V. Levin 2018-12-17 11:27 ` Oleg Nesterov 2018-12-22 9:54 ` Michael Ellerman 2 siblings, 1 reply; 20+ messages in thread From: Michael Ellerman @ 2018-12-17 11:20 UTC (permalink / raw) To: Dmitry V. Levin, Oleg Nesterov; +Cc: linuxppc-dev, linux-kernel "Dmitry V. Levin" <ldv@altlinux.org> writes: > Invoke tracehook_report_syscall_entry once. Thanks. > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > --- > arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++--------------- > 1 file changed, 31 insertions(+), 23 deletions(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index 714c3480c52d..8794d32c2d9e 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -3263,32 +3263,40 @@ static inline int do_seccomp(struct pt_regs *regs) { return 0; } > */ > long do_syscall_trace_enter(struct pt_regs *regs) > { > + u32 cached_flags; > + Do you mind if I just call it "flags", I find "cached_flags" a bit unwieldy for some reason. I'm happy to fix it up when applying. cheers > user_exit(); > > - if (test_thread_flag(TIF_SYSCALL_EMU)) { > - /* > - * A nonzero return code from tracehook_report_syscall_entry() > - * tells us to prevent the syscall execution, but we are not > - * going to execute it anyway. > - * > - * Returning -1 will skip the syscall execution. We want to > - * avoid clobbering any register also, thus, not 'gotoing' > - * skip label. > - */ > - if (tracehook_report_syscall_entry(regs)) > - ; > - return -1; > - } > + cached_flags = READ_ONCE(current_thread_info()->flags) & > + (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE); > > - /* > - * The tracer may decide to abort the syscall, if so tracehook > - * will return !0. Note that the tracer may also just change > - * regs->gpr[0] to an invalid syscall number, that is handled > - * below on the exit path. > - */ > - if (test_thread_flag(TIF_SYSCALL_TRACE) && > - tracehook_report_syscall_entry(regs)) > - goto skip; > + if (cached_flags) { > + int rc = tracehook_report_syscall_entry(regs); > + > + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { > + /* > + * A nonzero return code from > + * tracehook_report_syscall_entry() tells us > + * to prevent the syscall execution, but > + * we are not going to execute it anyway. > + * > + * Returning -1 will skip the syscall execution. > + * We want to avoid clobbering any register also, > + * thus, not 'gotoing' skip label. > + */ > + return -1; > + } > + > + if (rc) { > + /* > + * The tracer decided to abort the syscall. > + * Note that the tracer may also just change > + * regs->gpr[0] to an invalid syscall number, > + * that is handled below on the exit path. > + */ > + goto skip; > + } > + } > > /* Run seccomp after ptrace; allow it to set gpr[3]. */ > if (do_seccomp(regs)) > -- > ldv ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter 2018-12-17 11:20 ` Michael Ellerman @ 2018-12-17 11:23 ` Dmitry V. Levin 0 siblings, 0 replies; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-17 11:23 UTC (permalink / raw) To: Michael Ellerman; +Cc: linuxppc-dev, Oleg Nesterov, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1021 bytes --] Hi, On Mon, Dec 17, 2018 at 10:20:26PM +1100, Michael Ellerman wrote: > "Dmitry V. Levin" <ldv@altlinux.org> writes: > > Invoke tracehook_report_syscall_entry once. > > Thanks. > > > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> > > --- > > arch/powerpc/kernel/ptrace.c | 54 +++++++++++++++++++++--------------- > > 1 file changed, 31 insertions(+), 23 deletions(-) > > > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > > index 714c3480c52d..8794d32c2d9e 100644 > > --- a/arch/powerpc/kernel/ptrace.c > > +++ b/arch/powerpc/kernel/ptrace.c > > @@ -3263,32 +3263,40 @@ static inline int do_seccomp(struct pt_regs *regs) { return 0; } > > */ > > long do_syscall_trace_enter(struct pt_regs *regs) > > { > > + u32 cached_flags; > > + > > Do you mind if I just call it "flags", I find "cached_flags" a bit > unwieldy for some reason. > > I'm happy to fix it up when applying. No problem, feel free to call it whatever you like. Thanks, -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter 2018-12-16 17:28 ` [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter Dmitry V. Levin 2018-12-17 11:20 ` Michael Ellerman @ 2018-12-17 11:27 ` Oleg Nesterov 2018-12-22 9:54 ` Michael Ellerman 2 siblings, 0 replies; 20+ messages in thread From: Oleg Nesterov @ 2018-12-17 11:27 UTC (permalink / raw) To: Dmitry V. Levin; +Cc: linuxppc-dev, linux-kernel On 12/16, Dmitry V. Levin wrote: > > long do_syscall_trace_enter(struct pt_regs *regs) > { > + u32 cached_flags; > + > user_exit(); > > - if (test_thread_flag(TIF_SYSCALL_EMU)) { > - /* > - * A nonzero return code from tracehook_report_syscall_entry() > - * tells us to prevent the syscall execution, but we are not > - * going to execute it anyway. > - * > - * Returning -1 will skip the syscall execution. We want to > - * avoid clobbering any register also, thus, not 'gotoing' > - * skip label. > - */ > - if (tracehook_report_syscall_entry(regs)) > - ; > - return -1; > - } > + cached_flags = READ_ONCE(current_thread_info()->flags) & > + (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE); > > - /* > - * The tracer may decide to abort the syscall, if so tracehook > - * will return !0. Note that the tracer may also just change > - * regs->gpr[0] to an invalid syscall number, that is handled > - * below on the exit path. > - */ > - if (test_thread_flag(TIF_SYSCALL_TRACE) && > - tracehook_report_syscall_entry(regs)) > - goto skip; > + if (cached_flags) { > + int rc = tracehook_report_syscall_entry(regs); > + > + if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) { > + /* > + * A nonzero return code from > + * tracehook_report_syscall_entry() tells us > + * to prevent the syscall execution, but > + * we are not going to execute it anyway. > + * > + * Returning -1 will skip the syscall execution. > + * We want to avoid clobbering any register also, > + * thus, not 'gotoing' skip label. > + */ > + return -1; > + } > + > + if (rc) { > + /* > + * The tracer decided to abort the syscall. > + * Note that the tracer may also just change > + * regs->gpr[0] to an invalid syscall number, > + * that is handled below on the exit path. > + */ > + goto skip; > + } > + } Looks good to me, Oleg. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: powerpc/ptrace: cleanup do_syscall_trace_enter 2018-12-16 17:28 ` [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter Dmitry V. Levin 2018-12-17 11:20 ` Michael Ellerman 2018-12-17 11:27 ` Oleg Nesterov @ 2018-12-22 9:54 ` Michael Ellerman 2 siblings, 0 replies; 20+ messages in thread From: Michael Ellerman @ 2018-12-22 9:54 UTC (permalink / raw) To: Dmitry V. Levin, Oleg Nesterov; +Cc: linuxppc-dev, linux-kernel On Sun, 2018-12-16 at 17:28:28 UTC, "Dmitry V. Levin" wrote: > Invoke tracehook_report_syscall_entry once. > > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/8dbdec0bcb416d0ef0bfd737620d08 cheers ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [v5] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 15:56 ` [PATCH v5] " Dmitry V. Levin 2018-12-07 18:52 ` [PATCH v6] " Dmitry V. Levin @ 2018-12-11 13:45 ` Michael Ellerman 1 sibling, 0 replies; 20+ messages in thread From: Michael Ellerman @ 2018-12-11 13:45 UTC (permalink / raw) To: Dmitry V. Levin Cc: linux-kernel, Oleg Nesterov, Eugene Syromyatnikov, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev On Fri, 2018-12-07 at 15:56:05 UTC, "Dmitry V. Levin" wrote: > From: Elvira Khabirova <lineprinter@altlinux.org> > > Arch code should use tracehook_*() helpers, as documented > in include/linux/tracehook.h, > ptrace_report_syscall() is not expected to be used outside that file. > > The patch does not look very nice, but at least it is correct > and opens the way for PTRACE_GET_SYSCALL_INFO API. > > Co-authored-by: Dmitry V. Levin <ldv@altlinux.org> > Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU") > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: Breno Leitao <leitao@debian.org> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Eugene Syromyatnikov <esyr@redhat.com> > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org> > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/a225f1567405558fb5410e9b2b9080 cheers ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 15:42 ` Dmitry V. Levin 2018-12-07 15:56 ` [PATCH v5] " Dmitry V. Levin @ 2018-12-07 16:34 ` Oleg Nesterov 2018-12-07 18:42 ` Dmitry V. Levin 1 sibling, 1 reply; 20+ messages in thread From: Oleg Nesterov @ 2018-12-07 16:34 UTC (permalink / raw) To: Dmitry V. Levin Cc: Eugene Syromyatnikov, linux-kernel, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev On 12/07, Dmitry V. Levin wrote: > > On Fri, Dec 07, 2018 at 10:12:49PM +1100, Michael Ellerman wrote: > > > > Sorry, this patch does not work, please ignore it. > > > > Hmm OK. Why exactly? > > Unfortunately, I have no idea why it doesn't work. > All I can say is it breaks strace because the kernel no longer sends > syscall entry stops. May be because TIF_SYSCALL_EMU/etc is a bit number, not a mask? IOW, rather than whatever & TIF_XXX you should do whatever & _TIF_XXX intstead? Oleg. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call 2018-12-07 16:34 ` [PATCH v4] " Oleg Nesterov @ 2018-12-07 18:42 ` Dmitry V. Levin 0 siblings, 0 replies; 20+ messages in thread From: Dmitry V. Levin @ 2018-12-07 18:42 UTC (permalink / raw) To: Oleg Nesterov Cc: Eugene Syromyatnikov, linux-kernel, Elvira Khabirova, Paul Mackerras, Andy Lutomirski, Breno Leitao, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 773 bytes --] On Fri, Dec 07, 2018 at 05:34:10PM +0100, Oleg Nesterov wrote: > On 12/07, Dmitry V. Levin wrote: > > On Fri, Dec 07, 2018 at 10:12:49PM +1100, Michael Ellerman wrote: > > > > > > Sorry, this patch does not work, please ignore it. > > > > > > Hmm OK. Why exactly? > > > > Unfortunately, I have no idea why it doesn't work. > > All I can say is it breaks strace because the kernel no longer sends > > syscall entry stops. > > May be because TIF_SYSCALL_EMU/etc is a bit number, not a mask? IOW, rather > than > > whatever & TIF_XXX > > you should do > > whatever & _TIF_XXX > > intstead? Thanks Oleg, this was exactly the reason why it didn't work. That kind of things happens when you let userspace people hack you kernel. :) -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-12-22 10:37 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-16 11:17 [PATCH v2] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call Elvira Khabirova 2018-11-16 12:42 ` Michael Ellerman 2018-11-19 21:01 ` [PATCH v3] " Dmitry V. Levin 2018-11-21 21:17 ` Michael Ellerman 2018-12-03 3:18 ` [PATCH v4] " Dmitry V. Levin 2018-12-07 1:19 ` Dmitry V. Levin 2018-12-07 11:12 ` Michael Ellerman 2018-12-07 15:42 ` Dmitry V. Levin 2018-12-07 15:56 ` [PATCH v5] " Dmitry V. Levin 2018-12-07 18:52 ` [PATCH v6] " Dmitry V. Levin 2018-12-10 13:28 ` Oleg Nesterov 2018-12-10 13:36 ` Dmitry V. Levin 2018-12-16 17:28 ` [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter Dmitry V. Levin 2018-12-17 11:20 ` Michael Ellerman 2018-12-17 11:23 ` Dmitry V. Levin 2018-12-17 11:27 ` Oleg Nesterov 2018-12-22 9:54 ` Michael Ellerman 2018-12-11 13:45 ` [v5] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call Michael Ellerman 2018-12-07 16:34 ` [PATCH v4] " Oleg Nesterov 2018-12-07 18:42 ` Dmitry V. Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).