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

[Oleg Nesterov]

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.

As for the naming... I would like to suggest a better name for the new helper
but I can't ;)

Oleg.