Re: [PATCH] signal: add a helper for the si_code kernel impersonation check

[Oleg Nesterov]

On 08/05, Bradley Morgan wrote:
>
> On 5 August 2026 20:26:47 BST, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >On 07/06, Bradley Morgan wrote:
> >>
> >> +/*
> >> + * si_code values >= 0 and SI_TKILL are reserved to the kernel. Not
> >even
> >> + * root can use them to pretend a signal was sent by the kernel or by
> >> + * kill()/tgkill(), except when signaling itself.
> >> + */
> >> +static bool si_code_reserved_to_kernel(int si_code)
> >> +{
> >> +	return si_code >= 0 || si_code == SI_TKILL;
> >> +}
> >
> >...
> >
> >>  static int do_rt_sigqueueinfo(pid_t pid, int sig, kernel_siginfo_t
> >*info)
> >>  {
> >> -	/* Not even root can pretend to send signals from the kernel.
> >> -	 * Nor can they impersonate a kill()/tgkill(), which adds source info.
> >> -	 */
> >> -	if ((info->si_code >= 0 || info->si_code == SI_TKILL) &&
> >> -	    (task_pid_vnr(current) != pid))
> >> +	if (si_code_reserved_to_kernel(info->si_code) &&
> >> +	    task_pid_vnr(current) != pid)
> >
> >Well, technically this (cosmetic) change is obviouly correct. And I do
> >agree
> >it makes sense to factor out these checks and (more importantly) the
> >comments.
> >
> >But. IMO, the new comment looks a bit worse.
> >
> >The old comment has "impersonate a kill()/tgkill(), which adds source
> >info"
> >and this in fact explains that si_code == 0 or si_code == SI_TKILL come
> >with
> >the real/valid .si_pid and .si_uid  and the reciever can trust them.
> >
>
> Hmm, what do you suggest?

Hmm. it seems I wasn't clear...

If nothing else you can keep the old comment:

	* Not even root can pretend to send signals from the kernel.
	* Nor can they impersonate a kill()/tgkill(), which adds source info.

IMO, no need to add "si_code values >= 0 and SI_TKILL ..." at the start, this
just mirrors what the new (and trivial!) helper does.

May be you can improve it a bit, something like

	* Not even root can pretend to send SI_FROMKERNEL() signals.
	* Nor can they impersonate kill()/tgkill(), which have si_pid/uid

but this is subjective and minor.

I am fine either way. Just IMO the old comment(s) was more useful.

Oleg.