Re: Bad return value for getpwnam_r et al

2014-03-06 Thread Todd C. Miller
On Thu, 06 Mar 2014 01:25:12 +0100, Ingo Schwarze wrote: having just committed the first of my patches for getpwent.c and getgrent.c which fixes the return values without touching errno, i'd like to move on regarding some of the remaining issues, so i'm sending another two independent

Re: Bad return value for getpwnam_r et al

2014-03-05 Thread Ingo Schwarze
Hi, having just committed the first of my patches for getpwent.c and getgrent.c which fixes the return values without touching errno, i'd like to move on regarding some of the remaining issues, so i'm sending another two independent patches: 1. Prevent close() and syslog() from stomping on

Re: Bad return value for getpwnam_r et al

2014-03-05 Thread Ingo Schwarze
Hi, Ingo Schwarze wrote on Thu, Mar 06, 2014 at 01:25:12AM +0100: 2. Prevent getpw{nam,uid}_r() from touching errno at all. (as suggested by kettenis@ and agreed in principle by deraadt@) That patch will follow in my next mail. Here it is. Note that this is not necessarily a

Re: Bad return value for getpwnam_r et al

2014-02-24 Thread Ingo Schwarze
Hi Stuart, Stuart Henderson wrote on Sun, Feb 23, 2014 at 02:22:47PM +: btw, here are results from a search for getpw(nam|uid)_r in unpacked ports source, after removing a bunch of autoconf checks etc. Wow, these are nearly a hundred ports. It is not feasible to audit all that. Besides,

Re: Bad return value for getpwnam_r et al

2014-02-24 Thread Mark Kettenis
Date: Mon, 24 Feb 2014 16:51:42 +0100 From: Ingo Schwarze schwa...@usta.de Hi Stuart, Stuart Henderson wrote on Sun, Feb 23, 2014 at 02:22:47PM +: btw, here are results from a search for getpw(nam|uid)_r in unpacked ports source, after removing a bunch of autoconf checks etc.

Re: Bad return value for getpwnam_r et al

2014-02-24 Thread Theo de Raadt
While I agree this is a step in the right direction, and probably not going to break stuff, I'm not sure this is the time to change this. It does change the behaviour, and some ports will end up in codepaths Agreed. This is postponed to after release. It didn't make the cutoff.

Re: Bad return value for getpwnam_r et al

2014-02-22 Thread Ingo Schwarze
Well, given the total lack of feeback on my two diffs, and given that time is running out fast, let's jump the gun: Theo de Raadt wrote on Thu, Feb 20, 2014 at 05:43:18PM -0700: kettenis@ wrote: Indeed. POSIX explicitly says: No function in this volume of POSIX.1-2008 shall set errno to

Re: Bad return value for getpwnam_r et al

2014-02-20 Thread Theo de Raadt
Date: Tue, 18 Feb 2014 17:01:24 +0100 From: Ingo Schwarze schwa...@usta.de So: we need to - clear errno in the success and no match cases, This is incorrect and shouldn't be done. When no error occurs, errno has to stay as it is. Indeed. POSIX explicitly says: No

Re: Bad return value for getpwnam_r et al

2014-02-20 Thread Ingo Schwarze
Hi Theo, Theo de Raadt wrote on Thu, Feb 20, 2014 at 05:43:18PM -0700: kettenis@ wrote: Indeed. POSIX explicitly says: No function in this volume of POSIX.1-2008 shall set errno to zero. The standard is slightly ambiguous on what getpwnam_r() should do, but the way I read it, it

Re: Bad return value for getpwnam_r et al

2014-02-19 Thread Ingo Schwarze
Hi, i investigated some more. It is even worse than we thought so far. Philip Guenther wrote on Wed, Feb 05, 2014 at 04:31:16PM -0800: There's more needed. There are actually three cases: 1) found a match: getpwnam(): return non-NULL (Linux and Solaris set errno to 0)

Re: Bad return value for getpwnam_r et al

2014-02-19 Thread Ingo Schwarze
Hi, so here is the second patch that i'd recommend to put into the release. It has to be applied on top of the first one. Ingo Schwarze wrote on Wed, Feb 19, 2014 at 05:57:13PM +0100: C. No matter whether they succeed or fail, both functions may clobber errno with irrelevant errors.

Re: Bad return value for getpwnam_r et al

2014-02-18 Thread Ingo Schwarze
Hi, sorry for being late to the party, i just started to look at this issue. Obviously, there are multiple bugs here, so as Todd suggested, i think the fixes will be done in multiple steps. Here, i'm preliminarily addressing the non-YP part. Philip Guenther wrote on Wed, Feb 05, 2014 at

Re: Bad return value for getpwnam_r et al

2014-02-18 Thread Philip Guenther
On Tue, 18 Feb 2014, Mark Kettenis wrote: Date: Tue, 18 Feb 2014 17:01:24 +0100 From: Ingo Schwarze schwa...@usta.de So: we need to - clear errno in the success and no match cases, This is incorrect and shouldn't be done. When no error occurs, errno has to stay as it is.

Re: Bad return value for getpwnam_r et al

2014-02-18 Thread Philip Guenther
On Tue, 18 Feb 2014, Ingo Schwarze wrote: Index: gen/getpwent.c === RCS file: /cvs/src/lib/libc/gen/getpwent.c,v retrieving revision 1.48 diff -u -p -r1.48 getpwent.c --- gen/getpwent.c 15 Nov 2013 22:32:55 -

Re: Bad return value for getpwnam_r et al

2014-02-18 Thread Ingo Schwarze
Hi, Philip Guenther wrote on Tue, Feb 18, 2014 at 08:55:00AM -0800: On Tue, 18 Feb 2014, Mark Kettenis wrote: Ingo Schwarze schwa...@usta.de wrote: Philip Guenther wrote: So: we need to - clear errno in the success and no match cases, This is incorrect and shouldn't be done. When no

Re: Bad return value for getpwnam_r et al

2014-02-18 Thread Stuart Henderson
On 2014/02/18 18:25, Ingo Schwarze wrote: Yes, i guess that behaviour would conform to POSIX. However, the way i read it, it is unspecified whether getpwnam_r() sets errno in addition to returning the error code. So we have two options: a) leave the errno setting inside getpwnam_r(),

Re: Bad return value for getpwnam_r et al

2014-02-17 Thread Philip Guenther
On Sun, 16 Feb 2014, Brad Smith wrote: On 07/02/14 2:15 PM, Philip Guenther wrote: ... Oof. If YP is enabled but an error occurs when trying to do the lookup (yp_match() returns an error other than YPERR_KEY) then we should set errno even when the later passwd file lookup return no

Re: Bad return value for getpwnam_r et al

2014-02-16 Thread Brad Smith
On 07/02/14 2:15 PM, Philip Guenther wrote: On Thu, 6 Feb 2014, Todd C. Miller wrote: I don't particularly like all the errno clearing but I suppose we either need to clear it or restore it so the user can distinguish between error and not found conditions. Yep. Since linux and solaris zero

Re: Bad return value for getpwnam_r et al

2014-02-07 Thread Brad Smith
On 05/02/14 7:31 PM, Philip Guenther wrote: 9On Wed, 5 Feb 2014, Ted Unangst wrote: On Wed, Feb 05, 2014 at 12:41, Brad Smith wrote: They do (but through errno). Of course there may be other places where the internal functions clobber errno before returning to the user code, but some fixes

Re: Bad return value for getpwnam_r et al

2014-02-07 Thread Philip Guenther
On Thu, 6 Feb 2014, Todd C. Miller wrote: I don't particularly like all the errno clearing but I suppose we either need to clear it or restore it so the user can distinguish between error and not found conditions. Yep. Since linux and solaris zero it on success/no match, I think we should

Re: Bad return value for getpwnam_r et al

2014-02-07 Thread Todd C. Miller
On Fri, 07 Feb 2014 11:15:28 -0800, Philip Guenther wrote: Oof. If YP is enabled but an error occurs when trying to do the lookup (yp_match() returns an error other than YPERR_KEY) then we should set errno even when the later passwd file lookup return no match. So my patch is wrong as it

Re: Bad return value for getpwnam_r et al

2014-02-06 Thread Todd C. Miller
I don't particularly like all the errno clearing but I suppose we either need to clear it or restore it so the user can distinguish between error and not found conditions. I do think we need some errno handling in the YP code. For example, the db get in __has_yppw() needs the same handling as

Re: Bad return value for getpwnam_r et al

2014-02-05 Thread Brad Smith
On 30/01/14 5:32 AM, Jérémie Courrèges-Anglas wrote: will...@25thandclement.com writes: Synopsis: Bad return value for getpwnam_r et al Category: 42 Environment: System : OpenBSD 5.4 Details : OpenBSD 5.4 (GENERIC.MP) #41: Tue Jul 30 15:30:02 MDT 2013

Re: Bad return value for getpwnam_r et al

2014-02-05 Thread Ted Unangst
On Wed, Feb 05, 2014 at 12:41, Brad Smith wrote: They do (but through errno). Of course there may be other places where the internal functions clobber errno before returning to the user code, but some fixes have been applied already (eg. rev 1.43 of getpwent.c). Is anyone actually looking

Re: Bad return value for getpwnam_r et al

2014-02-05 Thread Philip Guenther
9On Wed, 5 Feb 2014, Ted Unangst wrote: On Wed, Feb 05, 2014 at 12:41, Brad Smith wrote: They do (but through errno). Of course there may be other places where the internal functions clobber errno before returning to the user code, but some fixes have been applied already (eg. rev 1.43 of

Re: Bad return value for getpwnam_r et al

2014-02-05 Thread Brad Smith
On Wed, Feb 05, 2014 at 12:52:12PM -0500, Ted Unangst wrote: On Wed, Feb 05, 2014 at 12:41, Brad Smith wrote: They do (but through errno). Of course there may be other places where the internal functions clobber errno before returning to the user code, but some fixes have been applied