Re: [PATCH v8 3/7] kernel: Implement selective syscall userspace redirection
From: Eric W. Biederman <hidden>
Date: 2021-06-30 21:44:58
Also in:
linux-kselftest, lkml
Why does do_syscal_user_dispatch call do_exit(SIGSEGV) and do_exit(SIGSYS) instead of force_sig(SIGSEGV) and force_sig(SIGSYS)? Looking at the code these cases are not expected to happen, so I would be surprised if userspace depends on any particular behaviour on the failure path so I think we can change this. Is using do_exit in this way something you copied from seccomp? The reason I am asking is that by using do_exit you deprive userspace of the change to catch the signal handler and try and fix things. Also by using do_exit only a single thread of a multi-thread application is terminated which seems wrong. I am asking because I am going through the callers of do_exit so I can refactor things and clean things up and this use just looks wrong. Gabriel Krisman Bertazi [off-list ref] writes: <snip>
+bool do_syscall_user_dispatch(struct pt_regs *regs)
+{
+ struct syscall_user_dispatch *sd = ¤t->syscall_dispatch;
+ char state;
+
+ if (likely(instruction_pointer(regs) - sd->offset < sd->len))
+ return false;
+
+ if (unlikely(arch_syscall_is_vdso_sigreturn(regs)))
+ return false;
+
+ if (likely(sd->selector)) {
+ /*
+ * access_ok() is performed once, at prctl time, when
+ * the selector is loaded by userspace.
+ */
+ if (unlikely(__get_user(state, sd->selector)))
+ do_exit(SIGSEGV); ^^^^^^^^^^^^^^^^
I think it makes more sense if the code does:
if (unlikely(__get_user(state, sd->selector))) {
force_sig(SIGSEGV);
return true;
}
+ + if (likely(state == PR_SYS_DISPATCH_OFF)) + return false; + + if (state != PR_SYS_DISPATCH_ON) + do_exit(SIGSYS);
^^^^^^^^^^^^^^^
+ } + + sd->on_dispatch = true; + syscall_rollback(current, regs); + trigger_sigsys(regs); + + return true; +}
Eric