All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dmitry V. Levin" <ldv@altlinux.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Oleg Nesterov <oleg@redhat.com>,
	Eugene Syromyatnikov <esyr@redhat.com>,
	Elvira Khabirova <lineprinter@altlinux.org>,
	Paul Mackerras <paulus@samba.org>,
	Andy Lutomirski <luto@kernel.org>,
	Breno Leitao <leitao@debian.org>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: [PATCH v6] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
Date: Fri, 7 Dec 2018 21:52:26 +0300	[thread overview]
Message-ID: <20181207185226.GB31278@altlinux.org> (raw)
In-Reply-To: <20181207155605.GB28964@altlinux.org>

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

WARNING: multiple messages have this Message-ID (diff)
From: "Dmitry V. Levin" <ldv@altlinux.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Elvira Khabirova <lineprinter@altlinux.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Oleg Nesterov <oleg@redhat.com>, Breno Leitao <leitao@debian.org>,
	Andy Lutomirski <luto@kernel.org>,
	Eugene Syromyatnikov <esyr@redhat.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: [PATCH v6] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
Date: Fri, 7 Dec 2018 21:52:26 +0300	[thread overview]
Message-ID: <20181207185226.GB31278@altlinux.org> (raw)
In-Reply-To: <20181207155605.GB28964@altlinux.org>

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

  reply	other threads:[~2018-12-07 18:54 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-16 11:17 [PATCH v2] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call Elvira Khabirova
2018-11-16 11:17 ` Elvira Khabirova
2018-11-16 12:42 ` Michael Ellerman
2018-11-16 12:42   ` Michael Ellerman
2018-11-19 21:01   ` [PATCH v3] " Dmitry V. Levin
2018-11-19 21:01     ` Dmitry V. Levin
2018-11-21 21:17     ` Michael Ellerman
2018-11-21 21:17       ` Michael Ellerman
2018-12-03  3:18       ` [PATCH v4] " Dmitry V. Levin
2018-12-03  3:18         ` Dmitry V. Levin
2018-12-07  1:19         ` Dmitry V. Levin
2018-12-07  1:19           ` Dmitry V. Levin
2018-12-07 11:12           ` Michael Ellerman
2018-12-07 11:12             ` Michael Ellerman
2018-12-07 15:42             ` Dmitry V. Levin
2018-12-07 15:42               ` Dmitry V. Levin
2018-12-07 15:56               ` [PATCH v5] " Dmitry V. Levin
2018-12-07 15:56                 ` Dmitry V. Levin
2018-12-07 18:52                 ` Dmitry V. Levin [this message]
2018-12-07 18:52                   ` [PATCH v6] " Dmitry V. Levin
2018-12-10 13:28                   ` Oleg Nesterov
2018-12-10 13:28                     ` Oleg Nesterov
2018-12-10 13:36                     ` Dmitry V. Levin
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:23                             ` Dmitry V. Levin
2018-12-17 11:27                         ` Oleg Nesterov
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-11 13:45                   ` Michael Ellerman
2018-12-07 16:34               ` [PATCH v4] " Oleg Nesterov
2018-12-07 16:34                 ` Oleg Nesterov
2018-12-07 18:42                 ` Dmitry V. Levin
2018-12-07 18:42                   ` Dmitry V. Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181207185226.GB31278@altlinux.org \
    --to=ldv@altlinux.org \
    --cc=esyr@redhat.com \
    --cc=leitao@debian.org \
    --cc=lineprinter@altlinux.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=oleg@redhat.com \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.