Seccomp support for linux-m68k

[John Paul Adrian Glaubitz]

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 - glau...@debian.org
`. `' Freie Universitaet Berlin - glau...@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

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

> 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

[Finn Thain]

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

[Finn Thain]

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.

[Michael Schmitz]

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

[John Paul Adrian Glaubitz]

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]

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

[Eero Tamminen]

Hi,

On 3/22/20 12:18 AM, 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?

Besides VM / container programs, SSH and browsers use it also:
https://en.wikipedia.org/wiki/Seccomp

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

That would require changing large number of
upstream projects (including things like Chromium)
to:
* load it dynamically
* handling libseccomp missing

I think much easier would be to patch libseccomp
to build without arch support, and just return
errors to API calls:
https://github.com/seccomp/libseccomp/blob/master/src/arch.c#L100

> 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?

In general it seems to require:

* libseccomp: arch recognition & syscall
mapping support:

https://github.com/seccomp/libseccomp/blob/master/src/arch-parisc-syscalls.c

* kernel: Adding the new syscall number

* kernel: Calling seccomp on syscall invalidation path

* kernel: Potentially something related to 32-bit & ptrace


Which all seem fairly straightforward, but many of
the projects seem to be using seccomp-bpf, instead
of plain seccomp.

How's m68k BPF support?


- Eero

PS. Not related to seccomp, but Hatari's 030
emulation exception handling has been improved
a lot recently, e.g. exception addresses on auto-
increment instructions should be correct etc.

(I'm still seeing random errors on kernel syscall
interface, but I'm starting to wonder whether it's
related to something else than 030 emulation, as
Linux works fine under the same CPU core code
within WinUAE Amiga emulator.)

[Finn Thain]

On Sun, 22 Mar 2020, Eero Tamminen wrote:

>
> PS. Not related to seccomp, but Hatari's 030 emulation exception
> handling has been improved a lot recently, e.g. exception addresses on
> auto- increment instructions should be correct etc.
>

Nice! I would like to revisit that '030 Linux bug we discussed last year
(echo t > /proc/sysrq-trigger) but there are too many other bugs to fix
and not enough developers to fix them.

> (I'm still seeing random errors on kernel syscall interface, but I'm
> starting to wonder whether it's related to something else than 030
> emulation, as Linux works fine under the same CPU core code within
> WinUAE Amiga emulator.)
>

A bug in the WinUAE CPU core might not show up in Amiga Linux. The Atari
Linux kernel does some strange things with interrupts (like never masking
IPL 1) that isn't done on any other platform.

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

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

[Michael Schmitz]

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

[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?

Andreas.

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

[John Paul Adrian Glaubitz]

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]

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

[Michael Schmitz]

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

[Michael Schmitz]

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
>

[Michael Schmitz]

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

[Andreas Schwab]

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 Schwab]

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.

[Michael Schmitz]

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

[Michael Schmitz]

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

[Andreas Schwab]

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

[Andreas Schwab]

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?

[Michael Schmitz]

Hi Andreas,

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

[Michael Schmitz]

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

[Andreas Schwab]

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.