On 06/22, Bradley Morgan wrote:
>
> send_signal_locked() should not change the caller's siginfo. Make that
> part of the type and keep the local rewrite on its copy.
>
> Suggested-by: Oleg Nesterov <[email protected]>
Ah, sorry... I only suggested to change the signature of send_signal_locked()
and thus has_si_pid_and_uid(). Perhaps a broader change makes sense too, but
this conflicts with another (under discussion) series:
PATCH v2 3/3] signal: fix evasion of SA_IMMUTABLE signals
https://lore.kernel.org/all/[email protected]/
Now let me take another look at 1/2 ...
Oleg.
> Signed-off-by: Bradley Morgan <[email protected]>
> ---
> Changes since v1:
> - New patch from Oleg's suggestion.
> - Link to Oleg's suggestion:
>
> https://lore.kernel.org/all/[email protected]/T/#m5f8a2d54928efff41de539969b68149e1ec5fca4
>
> include/linux/signal.h | 2 +-
> include/trace/events/signal.h | 4 ++--
> kernel/signal.c | 20 +++++++++++---------
> 3 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/signal.h b/include/linux/signal.h
> index f19816832f05..a1ba8c5973c6 100644
> --- a/include/linux/signal.h
> +++ b/include/linux/signal.h
> @@ -283,7 +283,7 @@ extern int do_send_sig_info(int sig, struct
> kernel_siginfo *info,
> struct task_struct *p, enum pid_type type);
> extern int group_send_sig_info(int sig, struct kernel_siginfo *info,
> struct task_struct *p, enum pid_type type);
> -extern int send_signal_locked(int sig, struct kernel_siginfo *info,
> +extern int send_signal_locked(int sig, const struct kernel_siginfo *info,
> struct task_struct *p, enum pid_type type);
> extern int sigprocmask(int, sigset_t *, sigset_t *);
> extern void set_current_blocked(sigset_t *);
> diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
> index 1db7e4b07c01..05a46135ee34 100644
> --- a/include/trace/events/signal.h
> +++ b/include/trace/events/signal.h
> @@ -49,8 +49,8 @@ enum {
> */
> TRACE_EVENT(signal_generate,
>
> - TP_PROTO(int sig, struct kernel_siginfo *info, struct task_struct *task,
> - int group, int result),
> + TP_PROTO(int sig, const struct kernel_siginfo *info,
> + struct task_struct *task, int group, int result),
>
> TP_ARGS(sig, info, task, group, result),
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index d72d9be3a992..26e8b8e1d03c 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1037,7 +1037,7 @@ static inline bool legacy_queue(struct sigpending
> *signals, int sig)
> return (sig < SIGRTMIN) && sigismember(&signals->signal, sig);
> }
>
> -static int __send_signal_locked(int sig, struct kernel_siginfo *info,
> +static int __send_signal_locked(int sig, const struct kernel_siginfo *info,
> struct task_struct *t, enum pid_type type, bool
> force)
> {
> struct sigpending *pending;
> @@ -1154,7 +1154,7 @@ static int __send_signal_locked(int sig, struct
> kernel_siginfo *info,
> return ret;
> }
>
> -static inline bool has_si_pid_and_uid(struct kernel_siginfo *info)
> +static inline bool has_si_pid_and_uid(const struct kernel_siginfo *info)
> {
> bool ret = false;
> switch (siginfo_layout(info->si_signo, info->si_code)) {
> @@ -1178,10 +1178,11 @@ static inline bool has_si_pid_and_uid(struct
> kernel_siginfo *info)
> return ret;
> }
>
> -int send_signal_locked(int sig, struct kernel_siginfo *info,
> +int send_signal_locked(int sig, const struct kernel_siginfo *info,
> struct task_struct *t, enum pid_type type)
> {
> struct kernel_siginfo rewritten;
> + const struct kernel_siginfo *send_info = info;
> /* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
> bool force = false;
>
> @@ -1196,26 +1197,27 @@ int send_signal_locked(int sig, struct kernel_siginfo
> *info,
> struct user_namespace *t_user_ns;
>
> rewritten = *info;
> - info = &rewritten;
> + send_info = &rewritten;
>
> rcu_read_lock();
> t_user_ns = task_cred_xxx(t, user_ns);
> if (current_user_ns() != t_user_ns) {
> - kuid_t uid = make_kuid(current_user_ns(), info->si_uid);
> - info->si_uid = from_kuid_munged(t_user_ns, uid);
> + kuid_t uid = make_kuid(current_user_ns(),
> rewritten.si_uid);
> +
> + rewritten.si_uid = from_kuid_munged(t_user_ns, uid);
> }
> rcu_read_unlock();
>
> /* A kernel generated signal? */
> - force = (info->si_code == SI_KERNEL);
> + force = (rewritten.si_code == SI_KERNEL);
>
> /* From an ancestor pid namespace? */
> if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
> - info->si_pid = 0;
> + rewritten.si_pid = 0;
> force = true;
> }
> }
> - return __send_signal_locked(sig, info, t, type, force);
> + return __send_signal_locked(sig, send_info, t, type, force);
> }
>
> static void print_fatal_signal(int signr)
> --
> 2.53.0
>
