Re: raise(-1) succeeds for programs linked against libpthread

2020-10-13 Thread Ludovic Courtès
Samuel Thibault  skribis:

> Ludovic Courtès, le mar. 13 oct. 2020 15:41:37 +0200, a ecrit:
>> ‘pthread_kill’ passes the signal number to ‘_hurd_raise_signal’, which
>> assumes it is valid:
> [...]
>> I suppose that before calling ‘sigaddset’, it should check whether SIGNO
>> is within bounds, along the lines of:
>> 
>>   if (signo < 2 || signo >= _NSIG)
>> return EINVAL;
>> 
>> Does that make sense?
>
> Probably, yes. Why excluding SIGHUP?

Oops, an oversight: I was looking at ‘signum-generic.h’ and the first
definition is SIGINT (2).  :-)

Ludo’.



Re: raise(-1) succeeds for programs linked against libpthread

2020-10-13 Thread Samuel Thibault
Ludovic Courtès, le mar. 13 oct. 2020 15:41:37 +0200, a ecrit:
> ‘pthread_kill’ passes the signal number to ‘_hurd_raise_signal’, which
> assumes it is valid:
[...]
> I suppose that before calling ‘sigaddset’, it should check whether SIGNO
> is within bounds, along the lines of:
> 
>   if (signo < 2 || signo >= _NSIG)
> return EINVAL;
> 
> Does that make sense?

Probably, yes. Why excluding SIGHUP?

Samuel



raise(-1) succeeds for programs linked against libpthread

2020-10-13 Thread Ludovic Courtès
Hi!

(Cc: bug-hurd.)

Jan Nieuwenhuizen  skribis:

>>> #include 
>>>
>>> int
>>> main (void)
>>> {
>>>   if (!raise (-1))
>>> return 1;
>>>   
>>>   return 0;
>>> }
>>
>> I don’t know if it’s relevant here, but you should always use ‘-pthread’
>> both at compile time and link time:
>>
>>   gcc raise.c -pthread
>>
>> That typically defines a few macros that may or may not have an effect
>> on the code at hand.
>
> Ah...right.  Makes no difference, though:
>
> root@childhurd ~# guix environment --bootstrap --ad-hoc gcc-toolchain@7
> root@childhurd ~ [env]# gcc raise.c
> root@childhurd ~ [env]# ./a.out
> root@childhurd ~ [env]# echo $?
> 0
> root@childhurd ~ [env]# gcc raise.c -pthread
> root@childhurd ~ [env]# ./a.out
> User defined signal 2

Interesting!  In the second case, we’re using ‘__pthread_kill’ from
‘pt-kill.c’ (instead of ‘kill’).

The expected behavior is that ‘raise’ should return non-zero and EINVAL.

‘pthread_kill’ passes the signal number to ‘_hurd_raise_signal’, which
assumes it is valid:

--8<---cut here---start->8---
int
_hurd_raise_signal (struct hurd_sigstate *ss,
int signo, const struct hurd_signal_detail *detail)
{
  if (ss == NULL)
{
  ss = _hurd_self_sigstate ();
  __spin_lock (>lock);
}

  /* Mark SIGNO as pending to be delivered.  */
  __sigaddset (>pending, signo);
  ss->pending_data[signo] = *detail;
--8<---cut here---end--->8---

I suppose that before calling ‘sigaddset’, it should check whether SIGNO
is within bounds, along the lines of:

  if (signo < 2 || signo >= _NSIG)
return EINVAL;

Does that make sense?

Thanks,
Ludo’.