Re: [PATCH v8 3/7] kernel: Implement selective syscall userspace redirection

From: Eric W. Biederman
Date: Wed Jun 30 2021 - 17:45:01 EST



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 <krisman@xxxxxxxxxxxxx> writes:

<snip>

> +bool do_syscall_user_dispatch(struct pt_regs *regs)
> +{
> + struct syscall_user_dispatch *sd = &current->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