Add Eric.

OK, I agree, it seems we need a simple fix.

Acked-by: Oleg Nesterov <[email protected]>

-------------------------------------------------------------------------
But let me add some "offtopic" notes... Why do we actually need this fix?

kill_something_info(). But at first glance sys_kill/kill_something_info
can simply use SEND_SIG_NOINFO? If yes, this makes sense anyway, I will
re-check...

do_pidfd_send_signal(PIDFD_SIGNAL_PROCESS_GROUP) allows to call
kill_pgrp_info() if si_code < 0... Not that I think this would be better,
but we could move this "rewrite" logic into __kill_pgrp_info()...

Anything else needs this change? Most probably yes, but after the quick
grep I don't see other group senders with !is_si_special(info).

Eric, what do you think?

Oleg.

On 06/22, Bradley Morgan wrote:
>
> send_signal_locked() rewrites sender ids for the target namespace.
> Group sends reuse the same siginfo, so one recipient can affect the
> next.
>
> Copy the siginfo before changing it.
>
> Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
> Cc: [email protected]
> Signed-off-by: Bradley Morgan <[email protected]>
> ---
> Changes since v1:
> - No code changes in this patch.
> - Add patch 2 for Oleg's const suggestion.
> - Link to v1:
>   
> https://lore.kernel.org/all/[email protected]/T/#m89955d13f10807c316d34cc76680d690a2d95b31
>
>  kernel/signal.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index b9fc7be1a169..d72d9be3a992 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1181,6 +1181,7 @@ static inline bool has_si_pid_and_uid(struct 
> kernel_siginfo *info)
>  int send_signal_locked(int sig, struct kernel_siginfo *info,
>                      struct task_struct *t, enum pid_type type)
>  {
> +     struct kernel_siginfo rewritten;
>       /* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
>       bool force = false;
>
> @@ -1194,6 +1195,9 @@ int send_signal_locked(int sig, struct kernel_siginfo 
> *info,
>               /* SIGKILL and SIGSTOP is special or has ids */
>               struct user_namespace *t_user_ns;
>
> +             rewritten = *info;
> +             info = &rewritten;
> +
>               rcu_read_lock();
>               t_user_ns = task_cred_xxx(t, user_ns);
>               if (current_user_ns() != t_user_ns) {
> --
> 2.53.0
>


Reply via email to