Re: [Xen-devel] [PATCH] libxl: Be more careful with error handling in libxl__dm_runas_helper()

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2015-11-26 at 17:45 +0000, Ian Jackson wrote:
> Boris Ostrovsky writes ("[PATCH] libxl: Be more careful with error
> handling in libxl__dm_runas_helper()"):
> > getpwnam_r() has fairly complicated return rules. From man pages:
> > 
> > Â RETURN VALUE
> > ÂÂÂÂÂÂ...
> > ÂÂÂÂÂÂOn success, getpwnam_r() and getpwuid_r() return zero, and set
> > ÂÂÂÂÂÂ*result to pwd.ÂÂIf no matchingÂÂpassword record was found, these
> > ÂÂÂÂÂÂfunctions return 0 and store NULL in *result. In case of error,
> > ÂÂÂÂÂÂan error number is returned, and NULL is stored in *result.
> 
> I can't see anything in the SuS docs (which Ian C referred to) saying
> that *result is updated even on error.ÂÂSo I think you need to check
> the return value first, and only then *result.

Actually, the fourth para of description in [0] (starting "The getpwnam_r()
function shall...") ends:

    A null pointer shall be returned at the location pointed to by result on
    error or if the requested entry is not found.

I also just realised (long after everyone else, apparently) that this
function returns (+ve) Exxx values, not -1 and setting errno.

Together with combining getpwnam and getpwnam_r in the same doc it's almost
like someone was wilfully trying to make this function difficult to figure
out...

> 
> > Â ERRORS
> > ÂÂÂÂÂÂ0 or ENOENT or ESRCH or EBADF or EPERM or ...
> > ÂÂÂÂÂÂÂÂÂÂÂÂThe given name or uid was not found.
> > 
> > While it's not clear what ellipses are meant to be, the way we
> > currently
> > treat return values from getpwnam_r() is no sufficient. In fact, two of
> > my systems behave differently when username is not found: one returns
> > ENOENT and the other returns 0. Both set *result to NULL.
> 
> I don't know where all that stuff about ENOENT comes from.ÂÂI'm
> tempted to say this is a bug in your C library.ÂÂBut I don't mind
> treating ENOENT or ESRCH as ERROR_NOTFOUND.
> 
> I do mind treating EBADF or EPERM that way.

FWIW the Linux manpage which Boris has referred to says under NOTES:

    Experiments on various UNIX-like systems show that lotsÂÂof   Â
    different values occur in this situation: 0, ENOENT, EBADF, ESRCH,
    EWOULDBLOCK, EPERM, and probably others

"various" probably includes all sorts of random UNIX-like systems outside
the small set which are actually used with Xen. I'd be inclined to agree
with not including EBADF, EPERM in this way at least until an actual
platform running Xen is found which returns such errors (not suggesting
anyone proactively checking, but just that we should be prepared to accept
the possibility when someone notices and reports it).

Ian.

[0]Âhttp://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwnam.html

> 
> > While at it, also make sure we don't get stuck on ERANGE.
> 
> If you are going to do anything to this, you should use an
> exponentially increasing buffer size.ÂÂBear in mind that getpwnam_r is
> already inside the trust boundary.
> 
> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel