Re: Openat without die

[Bastien ROUCARIES]

Le mardi 11 janvier 2011 17:16:20, Eric Blake a écrit :
> On 01/11/2011 09:05 AM, Jim Meyering wrote:
> >> BTW do you agree with merging my fist two patch?
> > 
> > I haven't looked carefully, but this use of sprintf will
> > dereference NULL when malloc fails:
> > 
> > -      char *result = (bufsize < OPENAT_BUFFER_SIZE ? buf : xmalloc
> > (bufsize)); +      char *result = (bufsize < OPENAT_BUFFER_SIZE ? buf :
> > malloc (bufsize));
> > 
> >        sprintf (result, PROC_SELF_FD_FORMAT, fd, file);
> 
> The idea of making openat() fail with ENOMEM instead of calling
> xalloc_die() is reasonable to me (POSIX doesn't explicitly document
> ENOMEM as a typical error from openat(), but it DOES have a global
> statement that any standardized functions can return errno values not
> documented in the standard provided that the value returned is accurate
> to the error in question and more specific than any of the documented
> errors).

I have also corrected a bug openat not testing NULL path. I return EFAULT like 
my Linux here.

> 
> But like Jim said, it means you now have to do NULL checks throughout
> the rest of the openat implementation, in order to properly fail with
> ENOMEM instead of crashing.  If a caller really cares about detecting
> ENOMEM, they can call xalloc_die themselves (but more likely, if
> openat() fails with ENOMEM, an app that uses xmalloc will likely fail
> with xalloc_die shortly thereafter on the next attempted allocation,
> even if such allocation is preparing the error() message for the
> detected failure of openat()).

Revised patch here


> And fixing the xmalloc/malloc problem is only half the problem, you
> still have to address the problem of chdir() failures before you've
> completely made the openat() emulation robust.  I also agree with your
> approach of separating the two issues into separate patches.

I know 

Bastien

0002-Reject-early-NULL-path-and-empty-path-in-at-function.patch
Description: Text Data

0001-Avoid-xmalloc-in-openat-implementation.patch
Description: Text Data

0003-Bail-out-early-in-case-of-ENOMEM-in-openat_proc_name.patch
Description: Text Data