• Seccomp support for linux-m68k

    From John Paul Adrian Glaubitz@21:1/5 to John Paul Adrian Glaubitz on Tue Jul 21 17:20:01 2020
    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

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to All on Sat Jul 25 11:40:01 2020
    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


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Andreas Schwab@21:1/5 to Michael Schmitz on Sat Jul 25 14:30:02 2020
    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."

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From John Paul Adrian Glaubitz@21:1/5 to Michael Schmitz on Sat Jul 25 21:00:01 2020
    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

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From John Paul Adrian Glaubitz@21:1/5 to Michael Schmitz on Sun Jul 26 00:50:02 2020
    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

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to All on Sun Jul 26 03:30:01 2020
    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.


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to All on Sun Jul 26 03:40:02 2020
    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


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to All on Sun Jul 26 09:20:02 2020
    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

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Andreas Schwab@21:1/5 to Michael Schmitz on Sun Jul 26 13:30:03 2020
    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."

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to Andreas Schwab on Sun Jul 26 22:50:03 2020
    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.


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to Andreas Schwab on Sun Jul 26 23:10:02 2020
    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.


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Andreas Schwab@21:1/5 to Michael Schmitz on Sun Jul 26 23:30:02 2020
    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."

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to Andreas Schwab on Sun Jul 26 23:40:01 2020
    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.


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Michael Schmitz@21:1/5 to Andreas Schwab on Mon Jul 27 00:50:02 2020
    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.


    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Andreas Schwab@21:1/5 to Michael Schmitz on Mon Jul 27 09:00:02 2020
    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."

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)