This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #13724] Do not segfault in pthread_setname_np (x, NULL)
- From: Rich Felker <dalias at aerifal dot cx>
- To: Andreas Schwab <schwab at suse dot de>
- Cc: Carlos O'Donell <carlos at redhat dot com>, OndÅej BÃlka <neleai at seznam dot cz>, libc-alpha at sourceware dot org
- Date: Mon, 7 Oct 2013 10:50:10 -0400
- Subject: Re: [PATCH][BZ #13724] Do not segfault in pthread_setname_np (x, NULL)
- Authentication-results: sourceware.org; auth=none
- References: <20131003122009 dot GA8891 at domone dot podge> <524DCA52 dot 2030609 at redhat dot com> <20131007141928 dot GV20515 at brightrain dot aerifal dot cx> <mvmli25xhm3 dot fsf at hawking dot suse dot de>
On Mon, Oct 07, 2013 at 04:47:48PM +0200, Andreas Schwab wrote:
> 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).
OK, but I'm unclear on your position. You disagree with my opinion
that checking for NULL and returning an error is bad, but you also
disagree with returning EFAULT. Do you prefer then that a different
error be returned?
Rich