linux-m68k.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Seccomp support for linux-m68k
@ 2020-03-20  8:46 John Paul Adrian Glaubitz
  2020-03-20 22:49 ` Finn Thain
  2020-07-21 15:13 ` John Paul Adrian Glaubitz
  0 siblings, 2 replies; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-03-20  8:46 UTC (permalink / raw)
  To: linux-m68k; +Cc: Debian m68k, Helge Deller

Hi!

Would it be possible to add seccomp support for m68k in the kernel?

There are some packages like kscreensaver in Debian that require
libseccomp-dev and it would therefore be desirable if we could 
that library on Linux/m68k as well.

From what I have learned from Helge Deller who added seccomp for
hppa, it doesn't seem much that is necessary to get seccomp working
on an architecture.

So, if anyone could work on the kernel part, I could do the work on
libseccomp.

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-20  8:46 Seccomp support for linux-m68k John Paul Adrian Glaubitz
@ 2020-03-20 22:49 ` Finn Thain
  2020-03-20 22:59   ` John Paul Adrian Glaubitz
  2020-07-21 15:13 ` John Paul Adrian Glaubitz
  1 sibling, 1 reply; 24+ messages in thread
From: Finn Thain @ 2020-03-20 22:49 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz; +Cc: linux-m68k, Debian m68k, Helge Deller


On Fri, 20 Mar 2020, John Paul Adrian Glaubitz wrote:

> Hi!
> 
> Would it be possible to add seccomp support for m68k in the kernel?
> 
> There are some packages like kscreensaver in Debian that require 
> libseccomp-dev and it would therefore be desirable if we could that 
> library on Linux/m68k as well.
> 
> From what I have learned from Helge Deller who added seccomp for hppa, 
> it doesn't seem much that is necessary to get seccomp working on an 
> architecture.
> 
> So, if anyone could work on the kernel part, I could do the work on 
> libseccomp.
> 
> Thanks,
> Adrian
> 

I suspect (without evidence) that many m68k systems are actually virtual 
machines. And the need for container hosting on m68k seems negligible.

Therefore, there doesn't seem to be a lot of actual benefit from seccomp.

There are 17 architectures (out of 25) lacking seccomp support. This 
suggests that the portability issue around this missing feature can't 
easily be pinned on m68k.

That's not the case for certain other feature, where the m68k port could 
more easily be blamed for harming portability or generality (potentially 
creating work for others).

Just based on the numbers (below) the foremost would be tracehook and 
generic-idle-thread.

Also, arguably the most desirable features for m68k are those that might 
improve performance (e.g. jump-labels, eBPF-JIT, locking/*, generic VDSO 
for 680x0 etc.).

$ grep -c TODO Documentation/features/*/*/arch-support.txt | sort -t: -k2n
	Documentation/features/time/clockevents/arch-support.txt:1
	Documentation/features/time/modern-timekeeping/arch-support.txt:1
	Documentation/features/vm/numa-memblock/arch-support.txt:2
	Documentation/features/vm/THP/arch-support.txt:5
Documentation/features/core/tracehook/arch-support.txt:6
	Documentation/features/sched/numa-balancing/arch-support.txt:6
Documentation/features/core/generic-idle-thread/arch-support.txt:8
Documentation/features/locking/lockdep/arch-support.txt:9
Documentation/features/debug/kgdb/arch-support.txt:12
Documentation/features/debug/kprobes/arch-support.txt:13
Documentation/features/debug/kretprobes/arch-support.txt:14
Documentation/features/time/irq-time-acct/arch-support.txt:14
Documentation/features/core/jump-labels/arch-support.txt:15
Documentation/features/perf/kprobes-event/arch-support.txt:15
Documentation/features/time/virt-cpuacct/arch-support.txt:15
	Documentation/features/vm/TLB/arch-support.txt:16
Documentation/features/debug/gcov-profile-all/arch-support.txt:17
Documentation/features/io/dma-contiguous/arch-support.txt:17
Documentation/features/seccomp/seccomp-filter/arch-support.txt:17
Documentation/features/vm/pte_special/arch-support.txt:17
Documentation/features/core/eBPF-JIT/arch-support.txt:18
Documentation/features/debug/stackprotector/arch-support.txt:18
Documentation/features/debug/uprobes/arch-support.txt:18
Documentation/features/locking/queued-rwlocks/arch-support.txt:18
Documentation/features/vm/ELF-ASLR/arch-support.txt:18
Documentation/features/locking/queued-spinlocks/arch-support.txt:19
Documentation/features/time/context-tracking/arch-support.txt:19
Documentation/features/perf/perf-regs/arch-support.txt:20
Documentation/features/perf/perf-stackdump/arch-support.txt:20
Documentation/features/time/arch-tick-broadcast/arch-support.txt:20
Documentation/features/vm/ioremap_prot/arch-support.txt:20
Documentation/features/debug/kprobes-on-ftrace/arch-support.txt:21
Documentation/features/core/cBPF-JIT/arch-support.txt:22
Documentation/features/debug/KASAN/arch-support.txt:22
Documentation/features/debug/optprobes/arch-support.txt:22
Documentation/features/locking/cmpxchg-local/arch-support.txt:22
Documentation/features/sched/membarrier-sync-core/arch-support.txt:22
Documentation/features/vm/PG_uncached/arch-support.txt:23
Documentation/features/vm/huge-vmap/arch-support.txt:23
Documentation/features/debug/user-ret-profiler/arch-support.txt:24

m68k has a "TODO" against all of the above features, except for the 6 that 
I've indented. Some of these seem to be inapplicable (e.g. virt-cpuacct, 
perf-regs).

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-20 22:49 ` Finn Thain
@ 2020-03-20 22:59   ` John Paul Adrian Glaubitz
  2020-03-20 23:08     ` Finn Thain
  2020-03-21 22:18     ` Michael Schmitz
  0 siblings, 2 replies; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-03-20 22:59 UTC (permalink / raw)
  To: Finn Thain; +Cc: linux-m68k, Debian m68k, Helge Deller

On 3/20/20 11:49 PM, Finn Thain wrote:
> I suspect (without evidence) that many m68k systems are actually virtual 
> machines. And the need for container hosting on m68k seems negligible.

It isn't about security. It's about being able to build more packages
as some packages have started to make libseccomp support mandatory.

> Therefore, there doesn't seem to be a lot of actual benefit from seccomp.

I disagree for the aforementioned reasons.

> There are 17 architectures (out of 25) lacking seccomp support. This 
> suggests that the portability issue around this missing feature can't 
> easily be pinned on m68k.

The question is how many of these 17 architectures are actually supported
by Debian.

If you look at the build results for libseccomp in Debian, you can see that
alpha, ia64, m68k, sh and sparc64 are missing the feature, everyone else
supports it [1].

Adrian

> [1] https://buildd.debian.org/status/package.php?p=libseccomp&suite=sid

-- 
 .''`.  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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-20 22:59   ` John Paul Adrian Glaubitz
@ 2020-03-20 23:08     ` Finn Thain
  2020-03-21 22:18     ` Michael Schmitz
  1 sibling, 0 replies; 24+ messages in thread
From: Finn Thain @ 2020-03-20 23:08 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz; +Cc: linux-m68k, Debian m68k, Helge Deller


On Fri, 20 Mar 2020, John Paul Adrian Glaubitz wrote:

> The question is how many of these 17 architectures are actually 
> supported by Debian.
> 

I don't see how this issue relates to any particular distro. Perhaps we 
can drop the upstream Cc.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-20 22:59   ` John Paul Adrian Glaubitz
  2020-03-20 23:08     ` Finn Thain
@ 2020-03-21 22:18     ` Michael Schmitz
  2020-03-21 22:48       ` John Paul Adrian Glaubitz
  1 sibling, 1 reply; 24+ messages in thread
From: Michael Schmitz @ 2020-03-21 22:18 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, Finn Thain
  Cc: linux-m68k, Debian m68k, Helge Deller

Adrian,

Am 21.03.2020 um 11:59 schrieb John Paul Adrian Glaubitz:
> On 3/20/20 11:49 PM, Finn Thain wrote:
>> I suspect (without evidence) that many m68k systems are actually virtual
>> machines. And the need for container hosting on m68k seems negligible.
>
> It isn't about security. It's about being able to build more packages
> as some packages have started to make libseccomp support mandatory.

Is there a good technical reason for this decision? I suppose most of 
these packages are not about VM or container hosting?

What about checking at runtime for availability of the library, and 
disabling VM related functionality if it wasn't possible to load?

In the event that kernel support can't be avoided: I suppose there a git 
commit for Helge's hppa changes that would help gauge the effort 
required for implementing such support?

Cheers,

	Michael


>
>> Therefore, there doesn't seem to be a lot of actual benefit from seccomp.
>
> I disagree for the aforementioned reasons.
>
>> There are 17 architectures (out of 25) lacking seccomp support. This
>> suggests that the portability issue around this missing feature can't
>> easily be pinned on m68k.
>
> The question is how many of these 17 architectures are actually supported
> by Debian.
>
> If you look at the build results for libseccomp in Debian, you can see that
> alpha, ia64, m68k, sh and sparc64 are missing the feature, everyone else
> supports it [1].
>
> Adrian
>
>> [1] https://buildd.debian.org/status/package.php?p=libseccomp&suite=sid
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-21 22:18     ` Michael Schmitz
@ 2020-03-21 22:48       ` John Paul Adrian Glaubitz
  2020-03-21 23:01         ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-03-21 22:48 UTC (permalink / raw)
  To: Michael Schmitz, Finn Thain; +Cc: linux-m68k, Debian m68k, Helge Deller

On 3/21/20 11:18 PM, Michael Schmitz wrote:
> Am 21.03.2020 um 11:59 schrieb John Paul Adrian Glaubitz:
>> On 3/20/20 11:49 PM, Finn Thain wrote:
>>> I suspect (without evidence) that many m68k systems are actually virtual
>>> machines. And the need for container hosting on m68k seems negligible.
>>
>> It isn't about security. It's about being able to build more packages
>> as some packages have started to make libseccomp support mandatory.
> 
> Is there a good technical reason for this decision? I suppose most of these packages are not about VM or container hosting?

I don't know but I don't think I have a good case arguing against that
as multiple upstream projects are using it.

> What about checking at runtime for availability of the library, and disabling VM related functionality if it wasn't possible to load?
> 
> In the event that kernel support can't be avoided: I suppose there a git commit for Helge's hppa changes that would help gauge the effort required for implementing such support?

It doesn't seem to be much that's necessary:

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c90f06943e05519a87140dc407cf589c220aeedf

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=910cd32e552ea09caa89cdbe328e468979b030dd

Other architectures are similarly minimal:

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8855d608c145c1ca0e26f4da00741080bb49d80d

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d218af78492a36a4ae607c08fedfb59258440314

So, I think it's feasible to add minimal seccomp support for m68k.

PS: I'm going to set up the Amiga 500 with the xsurf500 soonish. Got all hardware
    that I need now.

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-21 22:48       ` John Paul Adrian Glaubitz
@ 2020-03-21 23:01         ` John Paul Adrian Glaubitz
  0 siblings, 0 replies; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-03-21 23:01 UTC (permalink / raw)
  To: Michael Schmitz, Finn Thain; +Cc: linux-m68k, Debian m68k, Helge Deller

On 3/21/20 11:48 PM, John Paul Adrian Glaubitz wrote:
> It doesn't seem to be much that's necessary:
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c90f06943e05519a87140dc407cf589c220aeedf
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=910cd32e552ea09caa89cdbe328e468979b030dd
> 
> Other architectures are similarly minimal:
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8855d608c145c1ca0e26f4da00741080bb49d80d
> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d218af78492a36a4ae607c08fedfb59258440314
> 
> So, I think it's feasible to add minimal seccomp support for m68k.

The commit for TILE also seems to be interesting:

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a0ddef81f4aeeeec3326f6b6a255d8ea13b41908

-- 
 .''`.  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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-03-20  8:46 Seccomp support for linux-m68k John Paul Adrian Glaubitz
  2020-03-20 22:49 ` Finn Thain
@ 2020-07-21 15:13 ` John Paul Adrian Glaubitz
  2020-07-25  9:29   ` Michael Schmitz
  1 sibling, 1 reply; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-07-21 15:13 UTC (permalink / raw)
  To: linux-m68k; +Cc: Debian m68k, Helge Deller

Hello!

On 3/20/20 9:46 AM, John Paul Adrian Glaubitz wrote:
> Would it be possible to add seccomp support for m68k in the kernel?
> 
> There are some packages like kscreensaver in Debian that require
> libseccomp-dev and it would therefore be desirable if we could 
> that library on Linux/m68k as well.
> 
>>From what I have learned from Helge Deller who added seccomp for
> hppa, it doesn't seem much that is necessary to get seccomp working
> on an architecture.
> 
> So, if anyone could work on the kernel part, I could do the work on
> libseccomp.
I just had another look at the topic and it seems with just need a minimal
patch to add SECCOMP and SECCOMP_FILTER support when looking at the changes
for riscv64 [1].

The most complex change seem to be the changes in entry.S to add some additional
checks for syscall numbers. I think we could just do this for m68k (and SH) as
well.

The userland land part is trivial as well, I actually added SuperH support to
libseccomp today which was rather easy but my pull request was rejected for the
time being due to SuperH not supporting SECCOMP_FILTER yet (only basic SECCOMP).

So, if someone could do the kernel pieces for m68k, I would work on the userspace
changes in libsseccomp.

Adrian

> [1] https://github.com/torvalds/linux/commit/5340627e3fe08030988bdda46dd86cd5d5fb7517
> [2] https://github.com/seccomp/libseccomp/pull/271

-- 
 .''`.  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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-21 15:13 ` John Paul Adrian Glaubitz
@ 2020-07-25  9:29   ` Michael Schmitz
  2020-07-25 11:55     ` Andreas Schwab
                       ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Michael Schmitz @ 2020-07-25  9:29 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, linux-m68k; +Cc: Debian m68k, Helge Deller

Hi Adrian,

Am 22.07.2020 um 03:13 schrieb John Paul Adrian Glaubitz:
> Hello!
>
> On 3/20/20 9:46 AM, John Paul Adrian Glaubitz wrote:
>> Would it be possible to add seccomp support for m68k in the kernel?
>>
>> There are some packages like kscreensaver in Debian that require
>> libseccomp-dev and it would therefore be desirable if we could
>> that library on Linux/m68k as well.
>>
>> >From what I have learned from Helge Deller who added seccomp for
>> hppa, it doesn't seem much that is necessary to get seccomp working
>> on an architecture.
>>
>> So, if anyone could work on the kernel part, I could do the work on
>> libseccomp.
> I just had another look at the topic and it seems with just need a minimal
> patch to add SECCOMP and SECCOMP_FILTER support when looking at the changes
> for riscv64 [1].
>
> The most complex change seem to be the changes in entry.S to add some additional
> checks for syscall numbers. I think we could just do this for m68k (and SH) as
> well.

Looking at your SH patch, I see no changes to check for syscall numbers, 
just a check of the syscall_trace_enter() return code added? Is that all 
that's needed for m68k as well?

What return code would we need to set on returning from an aborted 
syscall? (Without setting a specific one, -ENOSYS will be used by default.)

> The userland land part is trivial as well, I actually added SuperH support to
> libseccomp today which was rather easy but my pull request was rejected for the
> time being due to SuperH not supporting SECCOMP_FILTER yet (only basic SECCOMP).
>
> So, if someone could do the kernel pieces for m68k, I would work on the userspace
> changes in libsseccomp.

My earlier patch switching m68k to use syscall_trace_enter() is 
incomplete, please add the return call check

--- a/arch/m68k/kernel/entry.S
+++ b/arch/m68k/kernel/entry.S
@@ -167,6 +167,8 @@ do_trace_entry:
         jbsr    syscall_trace_enter
         RESTORE_SWITCH_STACK
         addql   #4,%sp
+       tstb    %d0
+       jne     ret_from_syscall
         movel   %sp@(PT_OFF_ORIG_D0),%d0
         cmpl    #NR_syscalls,%d0
         jcs     syscall

and add the same seccomp check you used in the SH syscall_trace_enter() 
patch, if returning -ENOSYS on filtered syscalls is appropriate.

Cheers,

	Michael


>
> Adrian
>
>> [1] https://github.com/torvalds/linux/commit/5340627e3fe08030988bdda46dd86cd5d5fb7517
>> [2] https://github.com/seccomp/libseccomp/pull/271
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-25  9:29   ` Michael Schmitz
@ 2020-07-25 11:55     ` Andreas Schwab
  2020-07-26  1:23       ` Michael Schmitz
  2020-07-25 18:54     ` John Paul Adrian Glaubitz
  2020-07-25 22:48     ` John Paul Adrian Glaubitz
  2 siblings, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-07-25 11:55 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

On Jul 25 2020, Michael Schmitz wrote:

> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -167,6 +167,8 @@ do_trace_entry:
>         jbsr    syscall_trace_enter
>         RESTORE_SWITCH_STACK
>         addql   #4,%sp
> +       tstb    %d0
> +       jne     ret_from_syscall

Why tstb and not tstl?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-25  9:29   ` Michael Schmitz
  2020-07-25 11:55     ` Andreas Schwab
@ 2020-07-25 18:54     ` John Paul Adrian Glaubitz
  2020-07-26  1:34       ` Michael Schmitz
  2020-07-25 22:48     ` John Paul Adrian Glaubitz
  2 siblings, 1 reply; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-07-25 18:54 UTC (permalink / raw)
  To: Michael Schmitz, linux-m68k; +Cc: Debian m68k, Helge Deller

On 7/25/20 11:29 AM, Michael Schmitz wrote:
> Looking at your SH patch, I see no changes to check for syscall numbers, just a check of the syscall_trace_enter() return code added? Is that all that's needed for m68k as well?

From my understanding, you basically check whether the syscall number is -1
and if that's the case, you just skip to ret_from_syscall. At least that's what
we're doing on SH in entry.S.

The rest of the handling is done in C code in kernel/ptrace.c in do_syscall_trace_enter:

if (secure_computing() == -1)
		return -1;


> What return code would we need to set on returning from an aborted syscall?
> (Without setting a specific one, -ENOSYS will be used by default.)

If we're talking about syscall_trace_enter(), it should be -1.

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-25  9:29   ` Michael Schmitz
  2020-07-25 11:55     ` Andreas Schwab
  2020-07-25 18:54     ` John Paul Adrian Glaubitz
@ 2020-07-25 22:48     ` John Paul Adrian Glaubitz
  2 siblings, 0 replies; 24+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-07-25 22:48 UTC (permalink / raw)
  To: Michael Schmitz, linux-m68k; +Cc: Debian m68k, Helge Deller

Hi Michael!

On 7/25/20 11:29 AM, Michael Schmitz wrote:
>> So, if someone could do the kernel pieces for m68k, I would work on the userspace
>> changes in libsseccomp.
> 
> My earlier patch switching m68k to use syscall_trace_enter() is incomplete, please add the return call check
> 
> --- a/arch/m68k/kernel/entry.S
> +++ b/arch/m68k/kernel/entry.S
> @@ -167,6 +167,8 @@ do_trace_entry:
>         jbsr    syscall_trace_enter
>         RESTORE_SWITCH_STACK
>         addql   #4,%sp
> +       tstb    %d0
> +       jne     ret_from_syscall
>         movel   %sp@(PT_OFF_ORIG_D0),%d0
>         cmpl    #NR_syscalls,%d0
>         jcs     syscall

Could you make a v2 of this patch which includes this change?

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-25 11:55     ` Andreas Schwab
@ 2020-07-26  1:23       ` Michael Schmitz
  2020-07-26 11:03         ` Andreas Schwab
  0 siblings, 1 reply; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26  1:23 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

Hi Andreas,

Am 25.07.2020 um 23:55 schrieb Andreas Schwab:
> On Jul 25 2020, Michael Schmitz wrote:
>
>> --- a/arch/m68k/kernel/entry.S
>> +++ b/arch/m68k/kernel/entry.S
>> @@ -167,6 +167,8 @@ do_trace_entry:
>>         jbsr    syscall_trace_enter
>>         RESTORE_SWITCH_STACK
>>         addql   #4,%sp
>> +       tstb    %d0
>> +       jne     ret_from_syscall
>
> Why tstb and not tstl?

No particular reason - I had seen testb used in the syscall entry code 
and had copied that. Missed the use of testl elsewhere in entry.S ...

Fixed in v2 of my patch.

Cheers,

	Michael


>
> Andreas.
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-25 18:54     ` John Paul Adrian Glaubitz
@ 2020-07-26  1:34       ` Michael Schmitz
  2020-07-26  7:13         ` Michael Schmitz
  2020-07-26 11:05         ` Andreas Schwab
  0 siblings, 2 replies; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26  1:34 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, linux-m68k; +Cc: Debian m68k, Helge Deller

Hi Adrian,

Am 26.07.2020 um 06:54 schrieb John Paul Adrian Glaubitz:
>> What return code would we need to set on returning from an aborted syscall?
>> (Without setting a specific one, -ENOSYS will be used by default.)
>
> If we're talking about syscall_trace_enter(), it should be -1.

OK, that's -EPERM. Reading the comment in asm/errno.h, -ENOSYS is not a 
legitimate return code for syscalls to use. I'll change the trace entry 
check to set -EPERM instead.

Andreas: could we preset -EPERM as return code on entering 
do_trace_entry to save another jump, possibly even without setting 
-ENOSYS before attempting the syscall, or would that break the syscall ABI?

Cheers,

	Michael


>
> Adrian
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26  1:34       ` Michael Schmitz
@ 2020-07-26  7:13         ` Michael Schmitz
  2020-07-26 11:05         ` Andreas Schwab
  1 sibling, 0 replies; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26  7:13 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, linux-m68k; +Cc: Debian m68k, Helge Deller

Am 26.07.2020 um 13:34 schrieb Michael Schmitz:
> Andreas: could we preset -EPERM as return code on entering
> do_trace_entry to save another jump, possibly even without setting
> -ENOSYS before attempting the syscall, or would that break the syscall ABI?

Never mind - -ENOSYS is needed for strace only, not the syscall proper 
so a slightly simpler version saving one jump is possible.

I'll send a final version for Geert to consider if your tests show this 
actually works as intended.

Cheers,

	Michael


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26  1:23       ` Michael Schmitz
@ 2020-07-26 11:03         ` Andreas Schwab
  2020-07-26 21:02           ` Michael Schmitz
  0 siblings, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-07-26 11:03 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

On Jul 26 2020, Michael Schmitz wrote:

> No particular reason - I had seen testb used in the syscall entry code and

Where?  That may be bugs.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26  1:34       ` Michael Schmitz
  2020-07-26  7:13         ` Michael Schmitz
@ 2020-07-26 11:05         ` Andreas Schwab
  2020-07-26 20:46           ` Michael Schmitz
  1 sibling, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-07-26 11:05 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

On Jul 26 2020, Michael Schmitz wrote:

> OK, that's -EPERM. Reading the comment in asm/errno.h, -ENOSYS is not a
> legitimate return code for syscalls to use.

ENOSYS is the correct error number for unimplemented syscalls.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 11:05         ` Andreas Schwab
@ 2020-07-26 20:46           ` Michael Schmitz
  2020-07-26 21:10             ` Andreas Schwab
  0 siblings, 1 reply; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26 20:46 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

Hi Andreas,

On 26/07/20 11:05 PM, Andreas Schwab wrote:
> On Jul 26 2020, Michael Schmitz wrote:
>
>> OK, that's -EPERM. Reading the comment in asm/errno.h, -ENOSYS is not a
>> legitimate return code for syscalls to use.
> ENOSYS is the correct error number for unimplemented syscalls.

Yes, but that wasn't my point. -ENOSYS is returned by the syscall 
dispatcher in entry.S for unimplemented syscalls, but should never be 
returned by syscalls that are implemented to avoid messing up syscall 
detection by user code.

What I attempt to do is support syscall filtering. Returning -ENOSYS in 
that case would run the risk of masking existing syscalls to user probe 
code just because the user process happens to have insufficient privileges.

Please correct me if that is actually the expected behaviour here.

Cheers,

     Michael


>
> Andreas.
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 11:03         ` Andreas Schwab
@ 2020-07-26 21:02           ` Michael Schmitz
  2020-07-26 21:08             ` Andreas Schwab
  0 siblings, 1 reply; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26 21:02 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

Hi Andreas,

On 26/07/20 11:03 PM, Andreas Schwab wrote:
> On Jul 26 2020, Michael Schmitz wrote:
>
>> No particular reason - I had seen testb used in the syscall entry code and
> Where?  That may be bugs.

Here:

ENTRY(ret_from_signal)
         movel   %curptr@(TASK_STACK),%a1
         tstb    %a1@(TINFO_FLAGS+2)
         jge     1f
         jbsr    syscall_trace_leave
1:      RESTORE_SWITCH_STACK
         addql   #4,%sp
....

and here:

ENTRY(system_call)
         SAVE_ALL_SYS

         GET_CURRENT(%d1)
         movel   %d1,%a1

         | save top of frame
         movel   %sp,%curptr@(TASK_THREAD+THREAD_ESP0)

         | syscall trace?
         tstb    %a1@(TINFO_FLAGS+2)
         jmi     do_trace_entry
         cmpl    #NR_syscalls,%d0
         jcc     badsys

....

plus when saving the FPU context on resume (the comment there suggests 
FPUs keep 8 bits of status that must be tested, so that should be OK).

Cheers,

     Michael

>
> Andreas.
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 21:02           ` Michael Schmitz
@ 2020-07-26 21:08             ` Andreas Schwab
  2020-07-26 21:39               ` Michael Schmitz
  0 siblings, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-07-26 21:08 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

On Jul 27 2020, Michael Schmitz wrote:

> Hi Andreas,
>
> On 26/07/20 11:03 PM, Andreas Schwab wrote:
>> On Jul 26 2020, Michael Schmitz wrote:
>>
>>> No particular reason - I had seen testb used in the syscall entry code and
>> Where?  That may be bugs.
>
> Here:
>
> ENTRY(ret_from_signal)
>         movel   %curptr@(TASK_STACK),%a1
>         tstb    %a1@(TINFO_FLAGS+2)
>         jge     1f
>         jbsr    syscall_trace_leave
> 1:      RESTORE_SWITCH_STACK
>         addql   #4,%sp
> ....
>
> and here:
>
> ENTRY(system_call)
>         SAVE_ALL_SYS
>
>         GET_CURRENT(%d1)
>         movel   %d1,%a1
>
>         | save top of frame
>         movel   %sp,%curptr@(TASK_THREAD+THREAD_ESP0)
>
>         | syscall trace?
>         tstb    %a1@(TINFO_FLAGS+2)
>         jmi     do_trace_entry
>         cmpl    #NR_syscalls,%d0
>         jcc     badsys
>

How is that relevant?  That is testing a single bit, of course.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 20:46           ` Michael Schmitz
@ 2020-07-26 21:10             ` Andreas Schwab
  2020-07-26 22:40               ` Michael Schmitz
  0 siblings, 1 reply; 24+ messages in thread
From: Andreas Schwab @ 2020-07-26 21:10 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

On Jul 27 2020, Michael Schmitz wrote:

> What I attempt to do is support syscall filtering. Returning -ENOSYS in
> that case would run the risk of masking existing syscalls to user probe 
> code just because the user process happens to have insufficient privileges.

What does ENOSYS have to do with privileges?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 21:08             ` Andreas Schwab
@ 2020-07-26 21:39               ` Michael Schmitz
  2020-07-27  6:35                 ` Andreas Schwab
  0 siblings, 1 reply; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26 21:39 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

Hi Andreas,

On 27/07/20 9:08 AM, Andreas Schwab wrote:
> On Jul 27 2020, Michael Schmitz wrote:
>
>> Hi Andreas,
>>
>> On 26/07/20 11:03 PM, Andreas Schwab wrote:
>>> On Jul 26 2020, Michael Schmitz wrote:
>>>
>>>> No particular reason - I had seen testb used in the syscall entry code and
>>> Where?  That may be bugs.
>> Here:
>>
>> ENTRY(ret_from_signal)
>>          movel   %curptr@(TASK_STACK),%a1
>>          tstb    %a1@(TINFO_FLAGS+2)
>>          jge     1f
>>          jbsr    syscall_trace_leave
>> 1:      RESTORE_SWITCH_STACK
>>          addql   #4,%sp
>> ....
>>
>> and here:
>>
>> ENTRY(system_call)
>>          SAVE_ALL_SYS
>>
>>          GET_CURRENT(%d1)
>>          movel   %d1,%a1
>>
>>          | save top of frame
>>          movel   %sp,%curptr@(TASK_THREAD+THREAD_ESP0)
>>
>>          | syscall trace?
>>          tstb    %a1@(TINFO_FLAGS+2)
>>          jmi     do_trace_entry
>>          cmpl    #NR_syscalls,%d0
>>          jcc     badsys
>>
> How is that relevant?  That is testing a single bit, of course.

No, tstb tests a byte operand. btst tests a bit. And testing the second 
byte of the thread info flags is just what we need if the trace bits are 
14 and 15.

(Had to look up the m68k programmer's reference and check to be sure, 
admittedly.)

Cheers,

     Michael

>
> Andreas.
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 21:10             ` Andreas Schwab
@ 2020-07-26 22:40               ` Michael Schmitz
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Schmitz @ 2020-07-26 22:40 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

Hi Andreas,

On 27/07/20 9:10 AM, Andreas Schwab wrote:
> On Jul 27 2020, Michael Schmitz wrote:
>
>> What I attempt to do is support syscall filtering. Returning -ENOSYS in
>> that case would run the risk of masking existing syscalls to user probe
>> code just because the user process happens to have insufficient privileges.
> What does ENOSYS have to do with privileges?

Nevermind - closer reading of the seccomp_filter docs reveals that I 
shouldn't overwrite the return code that might have been set by a 
filter. So the question of what return code to use is for the filter 
code to decide.

I'll revert back to the first version of the check.

Cheers,

     Michael


>
> Andreas.
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Seccomp support for linux-m68k
  2020-07-26 21:39               ` Michael Schmitz
@ 2020-07-27  6:35                 ` Andreas Schwab
  0 siblings, 0 replies; 24+ messages in thread
From: Andreas Schwab @ 2020-07-27  6:35 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: John Paul Adrian Glaubitz, linux-m68k, Debian m68k, Helge Deller

On Jul 27 2020, Michael Schmitz wrote:

> No, tstb tests a byte operand. btst tests a bit.

You can test a bit with tstb as well, as you can see here.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-07-27  6:35 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20  8:46 Seccomp support for linux-m68k John Paul Adrian Glaubitz
2020-03-20 22:49 ` Finn Thain
2020-03-20 22:59   ` John Paul Adrian Glaubitz
2020-03-20 23:08     ` Finn Thain
2020-03-21 22:18     ` Michael Schmitz
2020-03-21 22:48       ` John Paul Adrian Glaubitz
2020-03-21 23:01         ` John Paul Adrian Glaubitz
2020-07-21 15:13 ` John Paul Adrian Glaubitz
2020-07-25  9:29   ` Michael Schmitz
2020-07-25 11:55     ` Andreas Schwab
2020-07-26  1:23       ` Michael Schmitz
2020-07-26 11:03         ` Andreas Schwab
2020-07-26 21:02           ` Michael Schmitz
2020-07-26 21:08             ` Andreas Schwab
2020-07-26 21:39               ` Michael Schmitz
2020-07-27  6:35                 ` Andreas Schwab
2020-07-25 18:54     ` John Paul Adrian Glaubitz
2020-07-26  1:34       ` Michael Schmitz
2020-07-26  7:13         ` Michael Schmitz
2020-07-26 11:05         ` Andreas Schwab
2020-07-26 20:46           ` Michael Schmitz
2020-07-26 21:10             ` Andreas Schwab
2020-07-26 22:40               ` Michael Schmitz
2020-07-25 22:48     ` John Paul Adrian Glaubitz

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).