Re: [PATCH][BZ #13724] Do not segfault in pthread_setname_np (x, NULL)

[Andreas Schwab <schwab at suse dot de>]

Rich Felker <dalias@aerifal.cx> writes:

> On Thu, Oct 03, 2013 at 03:49:38PM -0400, Carlos O'Donell wrote:
>> On 10/03/2013 08:20 AM, OndÅej BÃlka wrote:
>> > Hi, this is another bug that could be handled quickly. Problem is if 
>> > pthread_setname_np (x, NULL) should return error code or segfault. 
>> > (see https://sourceware.org/bugzilla/show_bug.cgi?id=13724 )
>> > If first is desired then following patch does it. If we choose for
>> > second then close that bugzilla entry.
>> > 
>> > 	* sysdeps/unix/sysv/linux/pthread_setname.c (pthread_setname_np):
>> > 	Handle null.
>> > 
>> > diff --git a/nptl/sysdeps/unix/sysv/linux/pthread_setname.c b/nptl/sysdeps/unix/sysv/linux/pthread_setname.c
>> > index d6455dd..45eefa0 100644
>> > --- a/nptl/sysdeps/unix/sysv/linux/pthread_setname.c
>> > +++ b/nptl/sysdeps/unix/sysv/linux/pthread_setname.c
>> > @@ -34,6 +34,10 @@ pthread_setname_np (th, name)
>> >  {
>> >    const struct pthread *pd = (const struct pthread *) th;
>> >  
>> > +  /* Return EFAULT like pthread_getname_np(x, NULL, 16) does.  */
>> > +  if (name == NULL)
>> > +    return EFAULT;
>> > +
>> >    /* Unfortunately the kernel headers do not export the TASK_COMM_LEN
>> >       macro.  So we have to define it here.  */
>> >  #define TASK_COMM_LEN 16
>> > 
>> 
>> Thanks for tackling the BZs. You're doing a great job here.
>> 
>> I agree with Jeff Law on this one, this is not a performance
>> critical routine and checking arguments and returning EFAULT
>> is good for QoI.
>
> I disagree; I think it hurts QoI.

I agree to disagree.  Additionally, EFAULT is a kernel thing, it should
not be misused in user space (and its use in sysdeps/posix/waitid.c
should be removed).

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."