Re: [PATCH] signal: don't silently convert SI_USER signals to non-current pidfd

From: Christian Brauner
Date: Fri Mar 29 2019 - 22:17:42 EST


On Sat, Mar 30, 2019 at 03:12:32AM +0100, Jann Horn wrote:
> The current sys_pidfd_send_signal() silently turns signals with explicit
> SI_USER context that are sent to non-current tasks into signals with
> kernel-generated siginfo.
> This is unlike do_rt_sigqueueinfo(), which returns -EPERM in this case.
> If a user actually wants to send a signal with kernel-provided siginfo,
> they can do that with pidfd_send_signal(pidfd, sig, NULL, 0); so allowing
> this case is unnecessary.
>
> Instead of silently replacing the siginfo, just bail out with an error;
> this is consistent with other interfaces and avoids special-casing behavior
> based on security checks.
>
> Fixes: 3eb39f47934f ("signal: add pidfd_send_signal() syscall")
> Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>

Reviewed-by: Christian Brauner <christian@xxxxxxxxxx>

As discussed in
https://lore.kernel.org/lkml/20190330012229.yt3hecmgaj2r6vp7@xxxxxxxxxx
targeting this for a 5.1 rc.

> ---
> kernel/signal.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index b7953934aa99..f98448cf2def 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -3605,16 +3605,11 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
> if (unlikely(sig != kinfo.si_signo))
> goto err;
>
> + /* Only allow sending arbitrary signals to yourself. */
> + ret = -EPERM;
> if ((task_pid(current) != pid) &&
> - (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)) {
> - /* Only allow sending arbitrary signals to yourself. */
> - ret = -EPERM;
> - if (kinfo.si_code != SI_USER)
> - goto err;
> -
> - /* Turn this into a regular kill signal. */
> - prepare_kill_siginfo(sig, &kinfo);
> - }
> + (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL))
> + goto err;
> } else {
> prepare_kill_siginfo(sig, &kinfo);
> }
> --
> 2.21.0.392.gf8f6787159e-goog
>