syscall_trace_enter and syscall_trace_leave for m68k w/MMU

syscall_trace_enter and syscall_trace_leave for m68k w/MMU

[John Paul Adrian Glaubitz]

Hello!

Michael and I have been working on SECCOMP in SuperH and successfully added
the missing pieces yesterday [1].

We're now looking into what needs to be done for m68k and I have noticed that
syscall_trace_enter and syscall_trace_leave - which needs to be touched for
SECCOMP support - are available on ColdFire and non-MMU targets for m68k.

Is there a particular reason for that? Looking at 68000/entry.S, I can see
that both functions are invoked [2] so I'm not sure why they are built
for CONFIG_COLDFIRE and !CONFIG_MMM only.

Any ideas?

Adrian

> [1] https://marc.info/?l=linux-sh&m=159546012829989&w=2
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/m68k/68000/entry.S

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: syscall_trace_enter and syscall_trace_leave for m68k w/MMU

[Michael Schmitz]

Adrian,

m68k/mm only has syscall_trace() (in kernel/ptrace.c) which is called 
from the system call entry in kernel/entry.S (look for do_trace_* there).

syscall_trace() is called before and after syscall execution (if the 
syscall trace flag is set). Looking at the sparc 32 and 64 bit trace 
code for comparison, you might try replacing these calls by 
syscall_trace_enter() and syscall_trace_leave(). The current code in 
syscall_trace replicates the code in ptrace_report_syscall which will be 
called from syscall_trace_enter() and syscall_trace_leave(), so things 
ought to continue to function as normal.

syscall_trace() is also called from the ret_from_signal entry - no idea 
what should happen to that one. My guess is to treat this as syscall 
trace exit.

CC to Andreas - maybe there are subtleties with strace or gdb that I'm 
missing.

Cheers,

     Michael


On 23/07/20 10:03 PM, John Paul Adrian Glaubitz wrote:
> Hello!
>
> Michael and I have been working on SECCOMP in SuperH and successfully added
> the missing pieces yesterday [1].
>
> We're now looking into what needs to be done for m68k and I have noticed that
> syscall_trace_enter and syscall_trace_leave - which needs to be touched for
> SECCOMP support - are available on ColdFire and non-MMU targets for m68k.
>
> Is there a particular reason for that? Looking at 68000/entry.S, I can see
> that both functions are invoked [2] so I'm not sure why they are built
> for CONFIG_COLDFIRE and !CONFIG_MMM only.
>
> Any ideas?
>
> Adrian
>
>> [1] https://marc.info/?l=linux-sh&m=159546012829989&w=2
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/m68k/68000/entry.S

[PATCH RFC] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

m68k (other than Coldfire) uses syscall_trace for both trace entry
and trace exit. Seccomp support requires separate entry points for
trace entry and exit which are already provided for Coldfire.

Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
in preparation for seccomp support.

No regression seen in testing with strace on ARAnyM.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
---
 arch/m68k/kernel/entry.S  |    6 +++---
 arch/m68k/kernel/ptrace.c |   17 -----------------
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
index 9dd76fb..76badc4 100644
--- a/arch/m68k/kernel/entry.S
+++ b/arch/m68k/kernel/entry.S
@@ -164,7 +164,7 @@ do_trace_entry:
 	movel	#-ENOSYS,%sp@(PT_OFF_D0)| needed for strace
 	subql	#4,%sp
 	SAVE_SWITCH_STACK
-	jbsr	syscall_trace
+	jbsr	syscall_trace_enter
 	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 	movel	%sp@(PT_OFF_ORIG_D0),%d0
@@ -177,7 +177,7 @@ badsys:
 do_trace_exit:
 	subql	#4,%sp
 	SAVE_SWITCH_STACK
-	jbsr	syscall_trace
+	jbsr	syscall_trace_leave
 	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 	jra	.Lret_from_exception
@@ -186,7 +186,7 @@ ENTRY(ret_from_signal)
 	movel	%curptr@(TASK_STACK),%a1
 	tstb	%a1@(TINFO_FLAGS+2)
 	jge	1f
-	jbsr	syscall_trace
+	jbsr	syscall_trace_leave
 1:	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 /* on 68040 complete pending writebacks if any */
diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index 748c63b..b747a86 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -272,22 +272,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	return -EIO;
 }
 
-asmlinkage void syscall_trace(void)
-{
-	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				 ? 0x80 : 0));
-	/*
-	 * this isn't the same as continuing with a signal, but it will do
-	 * for normal use.  strace only continues with a signal if the
-	 * stopping signal is not SIGTRAP.  -brl
-	 */
-	if (current->exit_code) {
-		send_sig(current->exit_code, current, 1);
-		current->exit_code = 0;
-	}
-}
-
-#if defined(CONFIG_COLDFIRE) || !defined(CONFIG_MMU)
 asmlinkage int syscall_trace_enter(void)
 {
 	int ret = 0;
@@ -302,4 +286,3 @@ asmlinkage void syscall_trace_leave(void)
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall_exit(task_pt_regs(current), 0);
 }
-#endif /* CONFIG_COLDFIRE */
-- 
1.7.0.4

[PATCH RFC v2] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

m68k (other than Coldfire) uses syscall_trace for both trace entry
and trace exit. Seccomp support requires separate entry points for
trace entry and exit which are already provided for Coldfire.

Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
in preparation for seccomp support. Check return code of
syscall_trace_enter(), and skip syscall if nonzero. Return code
will be set to -EPERM in that case.

No regression seen in testing with strace on ARAnyM.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
---

Changes from v1:

- add return code check in do_trace_entry branch to enable syscall
  filtering (will return -EPERM)
- change to use testl for return code check (suggested by Andreas Schwab)
 
---
 arch/m68k/kernel/entry.S  |   11 ++++++++---
 arch/m68k/kernel/ptrace.c |   17 -----------------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
index 9dd76fb..fbd2222 100644
--- a/arch/m68k/kernel/entry.S
+++ b/arch/m68k/kernel/entry.S
@@ -164,20 +164,25 @@ do_trace_entry:
 	movel	#-ENOSYS,%sp@(PT_OFF_D0)| needed for strace
 	subql	#4,%sp
 	SAVE_SWITCH_STACK
-	jbsr	syscall_trace
+	jbsr	syscall_trace_enter
 	RESTORE_SWITCH_STACK
 	addql	#4,%sp
+	tstl	%d0
+	jne	denied
 	movel	%sp@(PT_OFF_ORIG_D0),%d0
 	cmpl	#NR_syscalls,%d0
 	jcs	syscall
 badsys:
 	movel	#-ENOSYS,%sp@(PT_OFF_D0)
 	jra	ret_from_syscall
+denied:
+	movel	#-EPERM,%sp@(PT_OFF_D0)
+	jra	ret_from_syscall
 
 do_trace_exit:
 	subql	#4,%sp
 	SAVE_SWITCH_STACK
-	jbsr	syscall_trace
+	jbsr	syscall_trace_leave
 	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 	jra	.Lret_from_exception
@@ -186,7 +191,7 @@ ENTRY(ret_from_signal)
 	movel	%curptr@(TASK_STACK),%a1
 	tstb	%a1@(TINFO_FLAGS+2)
 	jge	1f
-	jbsr	syscall_trace
+	jbsr	syscall_trace_leave
 1:	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 /* on 68040 complete pending writebacks if any */
diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index 748c63b..b747a86 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -272,22 +272,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	return -EIO;
 }
 
-asmlinkage void syscall_trace(void)
-{
-	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				 ? 0x80 : 0));
-	/*
-	 * this isn't the same as continuing with a signal, but it will do
-	 * for normal use.  strace only continues with a signal if the
-	 * stopping signal is not SIGTRAP.  -brl
-	 */
-	if (current->exit_code) {
-		send_sig(current->exit_code, current, 1);
-		current->exit_code = 0;
-	}
-}
-
-#if defined(CONFIG_COLDFIRE) || !defined(CONFIG_MMU)
 asmlinkage int syscall_trace_enter(void)
 {
 	int ret = 0;
@@ -302,4 +286,3 @@ asmlinkage void syscall_trace_leave(void)
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall_exit(task_pt_regs(current), 0);
 }
-#endif /* CONFIG_COLDFIRE */
-- 
1.7.0.4

[PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

m68k (other than Coldfire) uses syscall_trace for both trace entry
and trace exit. Seccomp support requires separate entry points for
trace entry and exit which are already provided for Coldfire.

Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
in preparation for seccomp support. Check return code of
syscall_trace_enter(), and skip syscall if nonzero. Return code
will be left at what had been set by by ptrace or seccomp.

No regression seen in testing with strace on ARAnyM.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
---

Changes from RFC v1:

- add return code check in do_trace_entry branch to enable syscall
  filtering (will return -EPERM)
- change to use testl for return code check (suggested by Andreas Schwab)

Changes from RFC v2:

- don't set return code of filtered syscall - seccomp may want to set
  that for use by calling process.

---
 arch/m68k/kernel/entry.S  |    8 +++++---
 arch/m68k/kernel/ptrace.c |   17 -----------------
 2 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/arch/m68k/kernel/entry.S b/arch/m68k/kernel/entry.S
index 9dd76fb..de3d54a 100644
--- a/arch/m68k/kernel/entry.S
+++ b/arch/m68k/kernel/entry.S
@@ -164,9 +164,11 @@ do_trace_entry:
 	movel	#-ENOSYS,%sp@(PT_OFF_D0)| needed for strace
 	subql	#4,%sp
 	SAVE_SWITCH_STACK
-	jbsr	syscall_trace
+	jbsr	syscall_trace_enter
 	RESTORE_SWITCH_STACK
 	addql	#4,%sp
+	tstl	%d0
+	jne	ret_from_syscall
 	movel	%sp@(PT_OFF_ORIG_D0),%d0
 	cmpl	#NR_syscalls,%d0
 	jcs	syscall
@@ -177,7 +179,7 @@ badsys:
 do_trace_exit:
 	subql	#4,%sp
 	SAVE_SWITCH_STACK
-	jbsr	syscall_trace
+	jbsr	syscall_trace_leave
 	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 	jra	.Lret_from_exception
@@ -186,7 +188,7 @@ ENTRY(ret_from_signal)
 	movel	%curptr@(TASK_STACK),%a1
 	tstb	%a1@(TINFO_FLAGS+2)
 	jge	1f
-	jbsr	syscall_trace
+	jbsr	syscall_trace_leave
 1:	RESTORE_SWITCH_STACK
 	addql	#4,%sp
 /* on 68040 complete pending writebacks if any */
diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index 748c63b..b747a86 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -272,22 +272,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	return -EIO;
 }
 
-asmlinkage void syscall_trace(void)
-{
-	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				 ? 0x80 : 0));
-	/*
-	 * this isn't the same as continuing with a signal, but it will do
-	 * for normal use.  strace only continues with a signal if the
-	 * stopping signal is not SIGTRAP.  -brl
-	 */
-	if (current->exit_code) {
-		send_sig(current->exit_code, current, 1);
-		current->exit_code = 0;
-	}
-}
-
-#if defined(CONFIG_COLDFIRE) || !defined(CONFIG_MMU)
 asmlinkage int syscall_trace_enter(void)
 {
 	int ret = 0;
@@ -302,4 +286,3 @@ asmlinkage void syscall_trace_leave(void)
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall_exit(task_pt_regs(current), 0);
 }
-#endif /* CONFIG_COLDFIRE */
-- 
1.7.0.4

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

Hi Michael!

On 7/27/20 6:19 AM, Michael Schmitz wrote:
> m68k (other than Coldfire) uses syscall_trace for both trace entry
> and trace exit. Seccomp support requires separate entry points for
> trace entry and exit which are already provided for Coldfire.
> 
> Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
> in preparation for seccomp support. Check return code of
> syscall_trace_enter(), and skip syscall if nonzero. Return code
> will be left at what had been set by by ptrace or seccomp.

Correct me if I'm wrong, but shouldn't the skip happen when the return
code is -1? At least that's what we're doing on SuperH and that seems
to be what other architectures are doing as well.

Also, shouldn't that part of the change not be part of the patch that
adds support for SECCOMP filter like in [1]? I don't think it makes
sense to add the return code check unless the rest of SECCOMP filter
has been implemented.

Adrian

> [1] https://github.com/glaubitz/linux/commit/2fa1e7b9ba5150bc12adaddc017d5a6b79f363e7

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Adrian,

On 27/07/20 10:03 PM, John Paul Adrian Glaubitz wrote:
> Hi Michael!
>
> On 7/27/20 6:19 AM, Michael Schmitz wrote:
>> m68k (other than Coldfire) uses syscall_trace for both trace entry
>> and trace exit. Seccomp support requires separate entry points for
>> trace entry and exit which are already provided for Coldfire.
>>
>> Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
>> in preparation for seccomp support. Check return code of
>> syscall_trace_enter(), and skip syscall if nonzero. Return code
>> will be left at what had been set by by ptrace or seccomp.
> Correct me if I'm wrong, but shouldn't the skip happen when the return
> code is -1? At least that's what we're doing on SuperH and that seems
> to be what other architectures are doing as well.

What other non-zero return codes do you expect syscall_trace_enter() to 
set, and what should the action in entry.S be?

(Note that according to my reading, your SH patch does not actually do 
what your description says. If syscall_trace_enter() returns a positive 
non-zero value, that value is _not_ used as changed syscall number. SH 
uses r3 for the syscall number, not r0).

As far as I can see, any non-zero return code should abort the syscall, 
so I just test for that (for simplicity). Our use of the 
tracehook_report_syscall_entry() return code (directly passed back from 
syscall_trace_enter()) doesn't leave much choice there, see comment in 
include/linux/tracehook.h.

If later on seccomp needs any specific action, it should be easy enough 
to change the syscall number (offset PT_OFF_ORIG_D0 on the stack) or 
syscall return code (offset PT_OFF_D0). There's code in kernel/ptrace.c 
to do that AFAICS.

Changing the behaviour of syscall_trace_enter() to match what other 
architectures do (return changed syscall number, not error code) is 
beyond the scope of this patch. I suspect the capability to change 
syscall numbers from ptrace code does predate the seccomp filter 
approach, and as m68k has never used it in the past, I don't see a point 
to add this now.

> Also, shouldn't that part of the change not be part of the patch that
> adds support for SECCOMP filter like in [1]? I don't think it makes
> sense to add the return code check unless the rest of SECCOMP filter
> has been implemented.

After replacing syscall_trace() by syscall_trace_enter() and 
syscall_trace_leave(), there is a return code provided by 
syscall_trace_enter() which we must check, hence I added the check at 
the same time as replacing syscall_trace() for non-Coldfire m68k.

(I note that the same check should probably be added to coldfire/entry.S.)

I can't test any of the later seccomp related stuff, so I'd rather leave 
that bit to someone else who can.

Cheers,

     Michael

>
> Adrian
>
>> [1] https://github.com/glaubitz/linux/commit/2fa1e7b9ba5150bc12adaddc017d5a6b79f363e7

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

On 7/27/20 10:48 PM, Michael Schmitz wrote:
> On 27/07/20 10:03 PM, John Paul Adrian Glaubitz wrote:
>> Hi Michael!
>>
>> On 7/27/20 6:19 AM, Michael Schmitz wrote:
>>> m68k (other than Coldfire) uses syscall_trace for both trace entry
>>> and trace exit. Seccomp support requires separate entry points for
>>> trace entry and exit which are already provided for Coldfire.
>>>
>>> Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
>>> in preparation for seccomp support. Check return code of
>>> syscall_trace_enter(), and skip syscall if nonzero. Return code
>>> will be left at what had been set by by ptrace or seccomp.
>> Correct me if I'm wrong, but shouldn't the skip happen when the return
>> code is -1? At least that's what we're doing on SuperH and that seems
>> to be what other architectures are doing as well.
> 
> What other non-zero return codes do you expect syscall_trace_enter() to set, and what should the action in entry.S be?

I just don't think we should do it any different than the other architectures
which explicitly compare the return value against -1, i.e. RISC-V and PA-RISC.

> (Note that according to my reading, your SH patch does not actually do what your description says. If syscall_trace_enter() returns a positive non-zero value, that value is _not_ used as changed syscall number. SH uses r3 for the syscall number, not r0).

You are right, of course. I somehow mixed that up. You're right, it checks
the return value of syscall_trace_enter and it should skip the syscall
if syscall_trace_enter returns -1.

> As far as I can see, any non-zero return code should abort the syscall, so I just test for that (for simplicity). Our use of the tracehook_report_syscall_entry() return code (directly passed back from syscall_trace_enter()) doesn't leave much choice there, see comment in include/linux/tracehook.h.

My point is to stay consistent with the other architectures.

> If later on seccomp needs any specific action, it should be easy enough to change the syscall number (offset PT_OFF_ORIG_D0 on the stack) or syscall return code (offset PT_OFF_D0). There's code in kernel/ptrace.c to do that AFAICS.
> 
> Changing the behaviour of syscall_trace_enter() to match what other architectures do (return changed syscall number, not error code) is beyond the scope of this patch. I suspect the capability to change syscall numbers from ptrace code does predate the seccomp filter approach, and as m68k has never used it in the past, I don't see a point to add this now.

Yes, I agree and my previous comment in this regard was wrong.

>> Also, shouldn't that part of the change not be part of the patch that
>> adds support for SECCOMP filter like in [1]? I don't think it makes
>> sense to add the return code check unless the rest of SECCOMP filter
>> has been implemented.
> 
> After replacing syscall_trace() by syscall_trace_enter() and syscall_trace_leave(), there is a return code provided by syscall_trace_enter() which we must check, hence I added the check at the same time as replacing syscall_trace() for non-Coldfire m68k.
> 
> (I note that the same check should probably be added to coldfire/entry.S.)
Do we actually need to check the return value unless we implement SECCOMP_FILTER?
(Which no doubt we will do ;-)). 

> I can't test any of the later seccomp related stuff, so I'd rather leave that bit to someone else who can.

I will work on the necessary changes for libseccomp this week, so that we can test
whether the libseccomp live tests pass correctly on a patched kernel.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

 	Hi Adrian,

On Mon, 27 Jul 2020, John Paul Adrian Glaubitz wrote:
> On 7/27/20 10:48 PM, Michael Schmitz wrote:
>> I can't test any of the later seccomp related stuff, so I'd rather leave that bit to someone else who can.
>
> I will work on the necessary changes for libseccomp this week, so that we can test
> whether the libseccomp live tests pass correctly on a patched kernel.

Any update on this?

With the below, you can run "make kselftest-all" (does not support O=!),
copy tools/testing/selftests/seccomp/seccomp_bpf and
tools/testing/selftests/seccomp/seccomp_benchmark to your m68k system,
and run the tests.  Of course they fail, as the actual seccomp support
hasn't been written yet...

From f5d325fadc43823f2a7ab2ff2e436da5ebb31565 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Wed, 26 Aug 2020 11:52:26 +0200
Subject: [PATCH] [WIP] selftests/seccomp: Add m68k support

FIXME

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
  tools/testing/selftests/seccomp/seccomp_bpf.c | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 7a6d40286a421c63..21cfcd3fec9ddc61 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -135,6 +135,8 @@ struct seccomp_data {
  #  define __NR_seccomp 337
  # elif defined(__sh__)
  #  define __NR_seccomp 372
+# elif defined(__mc68000__)
+#  define __NR_seccomp 380
  # else
  #  warning "seccomp syscall number unknown for this architecture"
  #  define __NR_seccomp 0xffff
@@ -1725,6 +1727,10 @@ TEST_F(TRACE_poke, getpid_runs_normally)
  # define ARCH_REGS	struct pt_regs
  # define SYSCALL_NUM	gpr[3]
  # define SYSCALL_RET	gpr[0]
+#elif defined(__mc68000__)
+# define ARCH_REGS	struct pt_regs
+# define SYSCALL_NUM	orig_d0
+# define SYSCALL_RET	d0
  #else
  # error "Do not know how to find your architecture's registers and syscalls"
  #endif
@@ -1748,7 +1754,7 @@ TEST_F(TRACE_poke, getpid_runs_normally)
  /* Use PTRACE_GETREGS and PTRACE_SETREGS when available. This is useful for
   * architectures without HAVE_ARCH_TRACEHOOK (e.g. User-mode Linux).
   */
-#if defined(__x86_64__) || defined(__i386__) || defined(__mips__)
+#if defined(__x86_64__) || defined(__i386__) || defined(__mips__) || defined(__mc68000)
  #define HAVE_GETREGS
  #endif

@@ -1797,7 +1803,8 @@ void change_syscall(struct __test_metadata *_metadata,

  #if defined(__x86_64__) || defined(__i386__) || defined(__powerpc__) || \
  	defined(__s390__) || defined(__hppa__) || defined(__riscv) || \
-	defined(__xtensa__) || defined(__csky__) || defined(__sh__)
+	defined(__xtensa__) || defined(__csky__) || defined(__sh__) || \
+	defined(__mc68000)
  	{
  		regs.SYSCALL_NUM = syscall;
  	}
-- 
2.17.1

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

Hi Geert!

On 8/26/20 1:18 PM, Geert Uytterhoeven wrote:
>> I will work on the necessary changes for libseccomp this week, so that we can test
>> whether the libseccomp live tests pass correctly on a patched kernel.
> 
> Any update on this?

Yes, I have rebased and updated the code:

> https://github.com/glaubitz/libseccomp/tree/m68k

It needs another review as one test is failing on x86_64 now. But that's
not difficult.

> With the below, you can run "make kselftest-all" (does not support O=!),
> copy tools/testing/selftests/seccomp/seccomp_bpf and
> tools/testing/selftests/seccomp/seccomp_benchmark to your m68k system,
> and run the tests.  Of course they fail, as the actual seccomp support
> hasn't been written yet...
> 
> From f5d325fadc43823f2a7ab2ff2e436da5ebb31565 Mon Sep 17 00:00:00 2001
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Date: Wed, 26 Aug 2020 11:52:26 +0200
> Subject: [PATCH] [WIP] selftests/seccomp: Add m68k support
> 
> FIXME

Okay, thanks.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

Hi Michael,

On Mon, Jul 27, 2020 at 6:19 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> m68k (other than Coldfire) uses syscall_trace for both trace entry
> and trace exit. Seccomp support requires separate entry points for
> trace entry and exit which are already provided for Coldfire.
>
> Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
> in preparation for seccomp support. Check return code of
> syscall_trace_enter(), and skip syscall if nonzero. Return code
> will be left at what had been set by by ptrace or seccomp.
>
> No regression seen in testing with strace on ARAnyM.
>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

Thanks for your patch!

> Changes from RFC v1:
>
> - add return code check in do_trace_entry branch to enable syscall
>   filtering (will return -EPERM)
> - change to use testl for return code check (suggested by Andreas Schwab)
>
> Changes from RFC v2:
>
> - don't set return code of filtered syscall - seccomp may want to set
>   that for use by calling process.

As your email subject didn't contain "v3", b4[*] insists on picking up
RFC v2, and refuses to pick up the latest version, even when forced
(msgid not present)...

The patch itself looks good to me. Does it make sense to apply it before
we have real seccomp support?

[*] https://people.kernel.org/monsieuricon/introducing-b4-and-patch-attestation

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

Hi Geert!

On 8/26/20 1:23 PM, Geert Uytterhoeven wrote:
>> Changes from RFC v1:
>>
>> - add return code check in do_trace_entry branch to enable syscall
>>   filtering (will return -EPERM)
>> - change to use testl for return code check (suggested by Andreas Schwab)
>>
>> Changes from RFC v2:
>>
>> - don't set return code of filtered syscall - seccomp may want to set
>>   that for use by calling process.
> 
> As your email subject didn't contain "v3", b4[*] insists on picking up
> RFC v2, and refuses to pick up the latest version, even when forced
> (msgid not present)...
> 
> The patch itself looks good to me. Does it make sense to apply it before
> we have real seccomp support?

Adding SECCOMP support itself doesn't take much:

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=c4637d475170ca0d99973efd07df727012db6cd1
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=0bb605c2c7f2b4b314b91510810b226de7f34fa1

Once syscall_trace_entry/leave have been made available, we just need to
add some Kconfig bits and the part which checks the return code of
syscall_trace_enter.

The rest is done by arch-agnostic code.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

 	Hi Adrian,

On Wed, 26 Aug 2020, John Paul Adrian Glaubitz wrote:
> On 8/26/20 1:23 PM, Geert Uytterhoeven wrote:
>>> Changes from RFC v1:
>>>
>>> - add return code check in do_trace_entry branch to enable syscall
>>>   filtering (will return -EPERM)
>>> - change to use testl for return code check (suggested by Andreas Schwab)
>>>
>>> Changes from RFC v2:
>>>
>>> - don't set return code of filtered syscall - seccomp may want to set
>>>   that for use by calling process.
>>
>> As your email subject didn't contain "v3", b4[*] insists on picking up
>> RFC v2, and refuses to pick up the latest version, even when forced
>> (msgid not present)...
>>
>> The patch itself looks good to me. Does it make sense to apply it before
>> we have real seccomp support?
>
> Adding SECCOMP support itself doesn't take much:
>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=c4637d475170ca0d99973efd07df727012db6cd1
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=0bb605c2c7f2b4b314b91510810b226de7f34fa1
>
> Once syscall_trace_entry/leave have been made available, we just need to
> add some Kconfig bits and the part which checks the return code of
> syscall_trace_enter.
>
> The rest is done by arch-agnostic code.

I came up with the following (basic seccomp, no seccomp filter), but
the seccomp_bpf selftest still fails all over the place...

From 231c23d07200cdd338dcf8d2adae8322ecef18f4 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Wed, 26 Aug 2020 14:24:58 +0200
Subject: [PATCH] [WIP] seccomp support

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
  arch/m68k/Kconfig                   | 14 ++++++++++++++
  arch/m68k/include/asm/Kbuild        |  1 +
  arch/m68k/include/asm/thread_info.h |  2 ++
  arch/m68k/kernel/ptrace.c           |  4 ++++
  4 files changed, 21 insertions(+)

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 93bbb74ea876d6f2..29ab228a9a721939 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -127,6 +127,20 @@ endmenu

  menu "Kernel Features"

+config SECCOMP
+	bool
+	prompt "Enable seccomp to safely compute untrusted bytecode"
+	help
+	  This kernel feature is useful for number crunching applications
+	  that may need to compute untrusted bytecode during their
+	  execution. By using pipes or other transports made available to
+	  the process as file descriptors supporting the read/write
+	  syscalls, it's possible to isolate those applications in
+	  their own address space using seccomp. Once seccomp is
+	  enabled via prctl(PR_SET_SECCOMP), it cannot be disabled
+	  and the task is only allowed to execute a few safe syscalls
+	  defined by each seccomp mode.
+
  endmenu

  if !MMU
diff --git a/arch/m68k/include/asm/Kbuild b/arch/m68k/include/asm/Kbuild
index 1bff55aa2d54e2ce..d9f0f283707ff352 100644
--- a/arch/m68k/include/asm/Kbuild
+++ b/arch/m68k/include/asm/Kbuild
@@ -4,4 +4,5 @@ generic-y += extable.h
  generic-y += kvm_para.h
  generic-y += local64.h
  generic-y += mcs_spinlock.h
+generic-y += seccomp.h
  generic-y += spinlock.h
diff --git a/arch/m68k/include/asm/thread_info.h b/arch/m68k/include/asm/thread_info.h
index 3689c6718c883d23..388c5c0f2cc905c4 100644
--- a/arch/m68k/include/asm/thread_info.h
+++ b/arch/m68k/include/asm/thread_info.h
@@ -63,6 +63,7 @@ static inline struct thread_info *current_thread_info(void)
  #define TIF_NOTIFY_RESUME	5	/* callback before returning to user */
  #define TIF_SIGPENDING		6	/* signal pending */
  #define TIF_NEED_RESCHED	7	/* rescheduling necessary */
+#define TIF_SECCOMP		8	/* secure computing */
  #define TIF_DELAYED_TRACE	14	/* single step a syscall */
  #define TIF_SYSCALL_TRACE	15	/* syscall trace active */
  #define TIF_MEMDIE		16	/* is terminating due to OOM killer */
@@ -71,6 +72,7 @@ static inline struct thread_info *current_thread_info(void)
  #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
  #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
  #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
+#define _TIF_SECCOMP		(1 << TIF_SECCOMP)
  #define _TIF_DELAYED_TRACE	(1 << TIF_DELAYED_TRACE)
  #define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
  #define _TIF_MEMDIE		(1 << TIF_MEMDIE)
diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index 74d58a82a1353180..ec5653b85dcdb4f9 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -20,6 +20,7 @@
  #include <linux/user.h>
  #include <linux/signal.h>
  #include <linux/tracehook.h>
+#include <linux/seccomp.h>

  #include <linux/uaccess.h>
  #include <asm/page.h>
@@ -277,6 +278,9 @@ asmlinkage int syscall_trace_enter(void)

  	if (test_thread_flag(TIF_SYSCALL_TRACE))
  		ret = tracehook_report_syscall_entry(task_pt_regs(current));
+
+	secure_computing_strict(task_pt_regs(current)->orig_d0);
+
  	return ret;
  }

-- 
2.17.1

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

Hi Geert!

On 8/26/20 2:32 PM, Geert Uytterhoeven wrote:
>  #include <linux/uaccess.h>
>  #include <asm/page.h>
> @@ -277,6 +278,9 @@ asmlinkage int syscall_trace_enter(void)
> 
>      if (test_thread_flag(TIF_SYSCALL_TRACE))
>          ret = tracehook_report_syscall_entry(task_pt_regs(current));
> +
> +    secure_computing_strict(task_pt_regs(current)->orig_d0);
> +

That part is outdated. It was removed in the second commit I posted, see:

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=0bb605c2c7f2b4b314b91510810b226de7f34fa1

Do you have the check of the return value already in syscall_trace_entry?

It should check for "-1" and jump to syscall_exit if true.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

Hi Adrian,

On Wed, Aug 26, 2020 at 2:35 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On 8/26/20 2:32 PM, Geert Uytterhoeven wrote:
> >  #include <linux/uaccess.h>
> >  #include <asm/page.h>
> > @@ -277,6 +278,9 @@ asmlinkage int syscall_trace_enter(void)
> >
> >      if (test_thread_flag(TIF_SYSCALL_TRACE))
> >          ret = tracehook_report_syscall_entry(task_pt_regs(current));
> > +
> > +    secure_computing_strict(task_pt_regs(current)->orig_d0);
> > +
>
> That part is outdated. It was removed in the second commit I posted, see:
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=0bb605c2c7f2b4b314b91510810b226de7f34fa1

That's the part which adds seccomp filter support.

> Do you have the check of the return value already in syscall_trace_entry?
>
> It should check for "-1" and jump to syscall_exit if true.

No, as that's needed only for seccomp filter support, AFAIU.

Have a look at arm, where seccomp filter support is optional, depending on ABI.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

Hi Geert!

On 8/26/20 2:38 PM, Geert Uytterhoeven wrote:
>> That part is outdated. It was removed in the second commit I posted, see:
>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=0bb605c2c7f2b4b314b91510810b226de7f34fa1
> 
> That's the part which adds seccomp filter support.
> 
>> Do you have the check of the return value already in syscall_trace_entry?
>>
>> It should check for "-1" and jump to syscall_exit if true.
> 
> No, as that's needed only for seccomp filter support, AFAIU.
> 
> Have a look at arm, where seccomp filter support is optional, depending on ABI.

As far as I know, filter support is mandatory these days. At least for SH, libseccomp
sent me away with my PR and told me to come back until the kernel has filter support.

Maybe I'm missing something but let's put Michael Karcher in the loop, he knows better
as he helped me with the kernel parts of SECCOMP on SH.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

 	Hi Adrian,

On Wed, 26 Aug 2020, John Paul Adrian Glaubitz wrote:
> On 8/26/20 2:38 PM, Geert Uytterhoeven wrote:
>>> That part is outdated. It was removed in the second commit I posted, see:
>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/sh?id=0bb605c2c7f2b4b314b91510810b226de7f34fa1
>>
>> That's the part which adds seccomp filter support.
>>
>>> Do you have the check of the return value already in syscall_trace_entry?
>>>
>>> It should check for "-1" and jump to syscall_exit if true.
>>
>> No, as that's needed only for seccomp filter support, AFAIU.
>>
>> Have a look at arm, where seccomp filter support is optional, depending on ABI.
>
> As far as I know, filter support is mandatory these days. At least for SH, libseccomp
> sent me away with my PR and told me to come back until the kernel has filter support.

OK.

> Maybe I'm missing something but let's put Michael Karcher in the loop, he knows better
> as he helped me with the kernel parts of SECCOMP on SH.

More work on top of the previous patch. It starts to look better:

    -FAILED: 13 / 86 tests passed.
    +FAILED: 35 / 86 tests passed.

But there are still fishy failures.  Most popular one is:

     Test exited normally instead of by signal (code: 0)

And things like:

     Expected 0 (0) == syscall(__NR_getpid) (705)
     Expected 0 (-270272472) == ptrace(PTRACE_GETREGS, tracee, 0, &regs) (0)



From 1ae515061575024081af930f4e5f9283910648de Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Wed, 26 Aug 2020 16:11:35 +0200
Subject: [PATCH] [WIP] seccomp filter support

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
  arch/m68k/68000/entry.S         |  2 ++
  arch/m68k/Kconfig               |  1 +
  arch/m68k/coldfire/entry.S      |  2 ++
  arch/m68k/include/asm/syscall.h | 41 +++++++++++++++++++++++++++++++++
  arch/m68k/kernel/ptrace.c       |  3 ++-
  5 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/68000/entry.S b/arch/m68k/68000/entry.S
index 259b3661b614168f..3526970e3c10535f 100644
--- a/arch/m68k/68000/entry.S
+++ b/arch/m68k/68000/entry.S
@@ -47,6 +47,8 @@ do_trace:
  	jbsr	syscall_trace_enter
  	RESTORE_SWITCH_STACK
  	addql	#4,%sp
+	tstl	%d0
+	jne	ret_from_exception
  	movel	%sp@(PT_OFF_ORIG_D0),%d1
  	movel	#-ENOSYS,%d0
  	cmpl	#NR_syscalls,%d1
diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 29ab228a9a721939..2166c9d84794a969 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -19,6 +19,7 @@ config M68K
  	select GENERIC_STRNCPY_FROM_USER if MMU
  	select GENERIC_STRNLEN_USER if MMU
  	select HAVE_AOUT if MMU
+	select HAVE_ARCH_SECCOMP_FILTER
  	select HAVE_ASM_MODVERSIONS
  	select HAVE_DEBUG_BUGVERBOSE
  	select HAVE_FUTEX_CMPXCHG if MMU && FUTEX
diff --git a/arch/m68k/coldfire/entry.S b/arch/m68k/coldfire/entry.S
index d43a02795a4a445e..13bf787968273165 100644
--- a/arch/m68k/coldfire/entry.S
+++ b/arch/m68k/coldfire/entry.S
@@ -92,6 +92,8 @@ ENTRY(system_call)
  	jbsr	syscall_trace_enter
  	RESTORE_SWITCH_STACK
  	addql	#4,%sp
+	tstl	%d0
+	jne	ret_from_exception
  	movel	%d3,%a0
  	jbsr	%a0@
  	movel	%d0,%sp@(PT_OFF_D0)		/* save the return value */
diff --git a/arch/m68k/include/asm/syscall.h b/arch/m68k/include/asm/syscall.h
index 465ac039be09a1b8..ac0f5d997be63b07 100644
--- a/arch/m68k/include/asm/syscall.h
+++ b/arch/m68k/include/asm/syscall.h
@@ -4,6 +4,47 @@

  #include <uapi/linux/audit.h>

+static inline long syscall_get_nr(struct task_struct *tsk,
+				  struct pt_regs *regs)
+{
+	return regs->orig_d0;
+}
+
+static inline void syscall_rollback(struct task_struct *task,
+				    struct pt_regs *regs)
+{
+	regs->d0 = regs->orig_d0;
+}
+
+static inline long syscall_get_return_value(struct task_struct *task,
+					    struct pt_regs *regs)
+{
+	return regs->d0;
+}
+
+static inline void syscall_set_return_value(struct task_struct *task,
+					    struct pt_regs *regs,
+					    int error, long val)
+{
+	regs->d0 = error ?: val;
+}
+
+static inline void syscall_get_arguments(struct task_struct *tsk,
+					 struct pt_regs *regs,
+					 unsigned long *args)
+{
+	memcpy(args, &regs->d1, 6 * sizeof(args[0]));
+}
+
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 unsigned int i, unsigned int n,
+					 const unsigned long *args)
+{
+	BUG_ON(i + n > 6);
+	memcpy(&regs->d1 + i, args, n * sizeof(args[0]));
+}
+
  static inline int syscall_get_arch(struct task_struct *task)
  {
  	return AUDIT_ARCH_M68K;
diff --git a/arch/m68k/kernel/ptrace.c b/arch/m68k/kernel/ptrace.c
index ec5653b85dcdb4f9..a3a0a230dcdfc953 100644
--- a/arch/m68k/kernel/ptrace.c
+++ b/arch/m68k/kernel/ptrace.c
@@ -279,7 +279,8 @@ asmlinkage int syscall_trace_enter(void)
  	if (test_thread_flag(TIF_SYSCALL_TRACE))
  		ret = tracehook_report_syscall_entry(task_pt_regs(current));

-	secure_computing_strict(task_pt_regs(current)->orig_d0);
+	if (secure_computing())
+		return -1;

  	return ret;
  }
-- 
2.17.1



Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Geert,

On 26/08/20 11:23 PM, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Mon, Jul 27, 2020 at 6:19 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> m68k (other than Coldfire) uses syscall_trace for both trace entry
>> and trace exit. Seccomp support requires separate entry points for
>> trace entry and exit which are already provided for Coldfire.
>>
>> Replace syscall_trace by syscall_trace_enter and syscall_trace_leave
>> in preparation for seccomp support. Check return code of
>> syscall_trace_enter(), and skip syscall if nonzero. Return code
>> will be left at what had been set by by ptrace or seccomp.
>>
>> No regression seen in testing with strace on ARAnyM.
>>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> Thanks for your patch!
Thanks for looking into it!
>
>> Changes from RFC v1:
>>
>> - add return code check in do_trace_entry branch to enable syscall
>>    filtering (will return -EPERM)
>> - change to use testl for return code check (suggested by Andreas Schwab)
>>
>> Changes from RFC v2:
>>
>> - don't set return code of filtered syscall - seccomp may want to set
>>    that for use by calling process.
> As your email subject didn't contain "v3", b4[*] insists on picking up
> RFC v2, and refuses to pick up the latest version, even when forced

Argh - I thought that since the first two were tagged 'RFC', I could 
just restart with v1.

Never heard of b4, sorry.

> (msgid not present)...
>
> The patch itself looks good to me. Does it make sense to apply it before
> we have real seccomp support?

As far as I understand Adrian, it's needed as a prerequisite (though I 
admit there needs to be another patch adding the actual seccomp bits as 
well).

With my rather limited test setup, this was all I could test. I've done 
enough damage with 'I think this is how it should work' patches to add 
untested stuff.

In my not-at-all-humble opinion, the whole issue of whether to trigger a 
syscall abort on any non-zero return code, or just -1, is quite academic 
so I'd rather keep the broader non-zero test (unless someone points to a 
clear-cut reason why this is a Bad Thing).

Cheers,

     Michael


>
> [*] https://people.kernel.org/monsieuricon/introducing-b4-and-patch-attestation
>
> Gr{oetje,eeting}s,
>
>                          Geert
>

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

Hi Michael,

On Thu, Aug 27, 2020 at 2:08 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 26/08/20 11:23 PM, Geert Uytterhoeven wrote:
> > As your email subject didn't contain "v3", b4[*] insists on picking up
> > RFC v2, and refuses to pick up the latest version, even when forced
>
> Argh - I thought that since the first two were tagged 'RFC', I could
> just restart with v1.
>
> Never heard of b4, sorry.

It's the new tool to download patches and patch series, adding
automatically any received *-by tags, and the Link: tag.  Makes my life
easier ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Geert,

On 27/08/20 9:19 PM, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Thu, Aug 27, 2020 at 2:08 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> On 26/08/20 11:23 PM, Geert Uytterhoeven wrote:
>>> As your email subject didn't contain "v3", b4[*] insists on picking up
>>> RFC v2, and refuses to pick up the latest version, even when forced
>> Argh - I thought that since the first two were tagged 'RFC', I could
>> just restart with v1.
>>
>> Never heard of b4, sorry.
> It's the new tool to download patches and patch series, adding
> automatically any received *-by tags, and the Link: tag.  Makes my life
> easier ;-)

I take that as read. Rather you than me :-)

Do you want me to resend this as v3? And any preference on the branch 
test (non-zero, or -1)?

Cheers,

     Michael


>
> Gr{oetje,eeting}s,
>
>                          Geert
>

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

Hi Michael,

On Thu, Aug 27, 2020 at 9:29 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 27/08/20 9:19 PM, Geert Uytterhoeven wrote:
> > On Thu, Aug 27, 2020 at 2:08 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> >> On 26/08/20 11:23 PM, Geert Uytterhoeven wrote:
> >>> As your email subject didn't contain "v3", b4[*] insists on picking up
> >>> RFC v2, and refuses to pick up the latest version, even when forced
> >> Argh - I thought that since the first two were tagged 'RFC', I could
> >> just restart with v1.
> >>
> >> Never heard of b4, sorry.
> > It's the new tool to download patches and patch series, adding
> > automatically any received *-by tags, and the Link: tag.  Makes my life
> > easier ;-)
>
> I take that as read. Rather you than me :-)
>
> Do you want me to resend this as v3? And any preference on the branch
> test (non-zero, or -1)?

No need to resend, unless there will be a v4 ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Geert,

working on entry.S recently, I was reminded of this one. It's never been 
applied, and I can't find a trace of it in my tree.

Not sure how far Adrian got with seccomp support testing, so I'm 
uncertain this is still of interest ...

Cheers,

     Michael


On 28/08/20 8:58 pm, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Thu, Aug 27, 2020 at 9:29 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> On 27/08/20 9:19 PM, Geert Uytterhoeven wrote:
>>> On Thu, Aug 27, 2020 at 2:08 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>>>> On 26/08/20 11:23 PM, Geert Uytterhoeven wrote:
>>>>> As your email subject didn't contain "v3", b4[*] insists on picking up
>>>>> RFC v2, and refuses to pick up the latest version, even when forced
>>>> Argh - I thought that since the first two were tagged 'RFC', I could
>>>> just restart with v1.
>>>>
>>>> Never heard of b4, sorry.
>>> It's the new tool to download patches and patch series, adding
>>> automatically any received *-by tags, and the Link: tag.  Makes my life
>>> easier ;-)
>> I take that as read. Rather you than me :-)
>>
>> Do you want me to resend this as v3? And any preference on the branch
>> test (non-zero, or -1)?
> No need to resend, unless there will be a v4 ;-)
>
> Gr{oetje,eeting}s,
>
>                          Geert
>

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[John Paul Adrian Glaubitz]

On 6/15/21 12:11 AM, Michael Schmitz wrote:
> working on entry.S recently, I was reminded of this one. It's never been applied, and I can't find a trace of it in my tree.
> 
> Not sure how far Adrian got with seccomp support testing, so I'm uncertain this is still of interest ...
I still have a fork of libseccomp with m68k support and my SH support patch
was merged upstream.

So, if you guys can get the kernel bits in place, I can take care of libseccomp.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Adrian,

On 15/06/21 11:04 am, John Paul Adrian Glaubitz wrote:
> On 6/15/21 12:11 AM, Michael Schmitz wrote:
>> working on entry.S recently, I was reminded of this one. It's never been applied, and I can't find a trace of it in my tree.
>>
>> Not sure how far Adrian got with seccomp support testing, so I'm uncertain this is still of interest ...
> I still have a fork of libseccomp with m68k support and my SH support patch
> was merged upstream.
>
> So, if you guys can get the kernel bits in place, I can take care of libseccomp.

Thanks - what (aside from my old patch) is still missing on the kernel 
side?

Cheers,

     Michael

>
> Adrian
>

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Geert Uytterhoeven]

Hi Michael,

On Tue, Jun 15, 2021 at 1:14 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 15/06/21 11:04 am, John Paul Adrian Glaubitz wrote:
> > On 6/15/21 12:11 AM, Michael Schmitz wrote:
> >> working on entry.S recently, I was reminded of this one. It's never been applied, and I can't find a trace of it in my tree.
> >>
> >> Not sure how far Adrian got with seccomp support testing, so I'm uncertain this is still of interest ...
> > I still have a fork of libseccomp with m68k support and my SH support patch
> > was merged upstream.
> >
> > So, if you guys can get the kernel bits in place, I can take care of libseccomp.
>
> Thanks - what (aside from my old patch) is still missing on the kernel
> side?

IIRC, it wasn't working well yet.  Reading the archives, due to some incorrect
return value somewhere?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Geert,

On 15/06/21 7:51 pm, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Tue, Jun 15, 2021 at 1:14 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> On 15/06/21 11:04 am, John Paul Adrian Glaubitz wrote:
>>> On 6/15/21 12:11 AM, Michael Schmitz wrote:
>>>> working on entry.S recently, I was reminded of this one. It's never been applied, and I can't find a trace of it in my tree.
>>>>
>>>> Not sure how far Adrian got with seccomp support testing, so I'm uncertain this is still of interest ...
>>> I still have a fork of libseccomp with m68k support and my SH support patch
>>> was merged upstream.
>>>
>>> So, if you guys can get the kernel bits in place, I can take care of libseccomp.
>> Thanks - what (aside from my old patch) is still missing on the kernel
>> side?
> IIRC, it wasn't working well yet.  Reading the archives, due to some incorrect
> return value somewhere?

As I recall it, it wasn't clear what the correct return value should be. 
Seccomp uses -1 to signal syscall abort, but there is considerable 
variation among architectures about what signals a syscall abort. I've 
seen tests for return code > NR_syscalls, != 0, or == -1. Some 
architectures allow the return code to be used as the new syscall number 
(no idea how that works, with the argument list unchanged?).

But  no matter - I'll change the test and resubmit.

Cheers,

     Michael

>
> Gr{oetje,eeting}s,
>
>                          Geert
>

Re: [PATCH] m68k/kernel - wire up syscall_trace_enter/leave for m68k

[Michael Schmitz]

Hi Adrian,

the deeper I dig into kernel support for seccomp, the nastier it gets. 
Skipping a syscall after checks in syscall_trace_enter was just the easy 
part.

I now have something that compiles and survives a boot test on ARAnyM, 
but it's been entirely untested otherwise.

Will send v4 of the entry.S patch, and send the remaining seccomp stuff 
as a separate patch.

Cheers,

     Michael




On 15/06/21 11:04 am, John Paul Adrian Glaubitz wrote:
> On 6/15/21 12:11 AM, Michael Schmitz wrote:
>> working on entry.S recently, I was reminded of this one. It's never been applied, and I can't find a trace of it in my tree.
>>
>> Not sure how far Adrian got with seccomp support testing, so I'm uncertain this is still of interest ...
> I still have a fork of libseccomp with m68k support and my SH support patch
> was merged upstream.
>
> So, if you guys can get the kernel bits in place, I can take care of libseccomp.
>
> Adrian
>

Re: syscall_trace_enter and syscall_trace_leave for m68k w/MMU

[Greg Ungerer]

Hi Adrian,

On 23/7/20 8:03 pm, John Paul Adrian Glaubitz wrote:
> Hello!
> 
> Michael and I have been working on SECCOMP in SuperH and successfully added
> the missing pieces yesterday [1].
> 
> We're now looking into what needs to be done for m68k and I have noticed that
> syscall_trace_enter and syscall_trace_leave - which needs to be touched for
> SECCOMP support - are available on ColdFire and non-MMU targets for m68k.
> 
> Is there a particular reason for that? Looking at 68000/entry.S, I can see
> that both functions are invoked [2] so I'm not sure why they are built
> for CONFIG_COLDFIRE and !CONFIG_MMM only.
> 
> Any ideas?

I am late to the party on this.

But the reason is historical more than anything else.
The time that m68knommu, which included ColdFire, spent as its own architecture
directory (arch/m68knommu) led to a few things like this diverging.
There is not technical reason.

Regards
Greg


> Adrian
> 
>> [1] https://marc.info/?l=linux-sh&m=159546012829989&w=2
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/m68k/68000/entry.S
> 

Re: syscall_trace_enter and syscall_trace_leave for m68k w/MMU

[John Paul Adrian Glaubitz]

Hi Greg!

On 8/5/20 2:23 PM, Greg Ungerer wrote:
> But the reason is historical more than anything else.
> The time that m68knommu, which included ColdFire, spent as its own architecture
> directory (arch/m68knommu) led to a few things like this diverging.
> There is not technical reason.

That's what I was already suspecting after finding the commit that merged
both into one directory :-).

Thanks for the confirmation.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913