Re: [PATCH v2 2/2] signal: make send_signal_locked() take const siginfo

[Bradley Morgan <include@grrlz.net>]

On June 23, 2026 11:39:05 AM GMT+01:00, Oleg Nesterov <oleg@redhat.com>
wrote:
>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 <oleg@redhat.com>
>
>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/ajVD6ZmiSQLxjj57@redhat.com/
>
>Now let me take another look at 1/2 ...
>
>Oleg.

Aww. My bad.

You may keep this shelved in case :)


>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>> ---
>> Changes since v1:
>> - New patch from Oleg's suggestion.
>> - Link to Oleg's suggestion:
>>  
>https://lore.kernel.org/all/0873AC4A-3CB2-4F7B-BFE6-75D855AD22DC@grrlz.net/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
>> 
>
>

Thanks!