Re: [PATCH 1/4] signals: SEND_SIG_NOINFO should be considered as SI_FROMUSER()

[Oleg Nesterov <oleg@redhat.com>]

On 10/05, Sukadev Bhattiprolu wrote:
>
> Oleg Nesterov [oleg@redhat.com] wrote:
> | >From __send_signal()'s pov they mean
> |
> | 	SEND_SIG_NOINFO		from user
>
> Just to complicate further, all 'SEND_SIG_NOINFO' signals are from user,
> but not all 'from user' signals are SEND_SIG_NOINFO.

Yes, SEND_SIG_NOINFO means: sent by kernel on behalf of some process.

>
> | 	SEND_SIG_PRIV		from kernel
>
> SEND_SIG_PRIV also means there is no real info, just that sender is
> privileged.

Well. Unlike SEND_SIG_FORCED, SEND_SIG_NOINFO/SEND_SIG_NOINFO ask
__send_signal() to allocate and queue "struct sigqueue". But
SEND_SIG_PRIV and SEND_SIG_NOINFO both mean the real info, jut
this info is filled by __send_signal().

> | 	SEND_SIG_FORCED		no info
>
> Are 'forced' signals considered 'from kernel' too ?

I think yes.

> | +static inline bool si_fromuser(const struct siginfo *info)
> | +{
> | +	return info == SEND_SIG_NOINFO ||
> | +		(!is_si_special(info) && SI_FROMUSER(info));
> | +}
> | +
>
> This change makes sense, but can we even drop the SEND_SIG_NOINFO
> altogether and simply check for NULL:
>
> 	return (!info || (is_si_special(info)) && SI_FROMUSER(info))

IOW, you suggest to use NULL instead of SEND_SIG_NOINFO.

Why?

If we use NULL as a "special" info, then SEND_SIG_FORCED semantics
makes more sense because __send_signal(SEND_SIG_FORCED) does not queue
info.

But I don't think we should use NULL. I think it is better to use the
symbolic names instead of NULL which is in fact the "harcoded constant".
But it would be nice to rename them.

Oleg.