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
