This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Remove __wur from setfsuid and setfsgid.


On Thu, Apr 18, 2013 at 07:22:20AM +0200, Florian Weimer wrote:
> On 04/18/2013 06:45 AM, Adam Conrad wrote:
> >This is a partial revert of 7e66ee5142deda977163d0a858c3d2883cae3f07,
> >which added __wur to ten set*id functions.
> >
> >The other eight functions have clear and sane returns that people
> >should be checking, however, the setfs*id functions do not, according
> >to their manpages:
> >
> >RETURN VALUE
> >        On success, the previous value of fsgid is returned.  On error,
> >        the current value of fsgid is returned.
> >
> >RETURN VALUE
> >        On success, the previous value of fsuid is returned.  On error,
> >        the current value of fsuid is returned.
> >
> >It was suggested in a Debian build failure bug[1] that these functions
> >shouldn't have the __wur attribute, and I'm inclined to agree.
> 
> Unfortunately, the prepare_cred() call in the kernel implementation
> allocates memory and calls into the LSM framework, so these
> functions can actually fail.  But it's true that it's impossible to
> detect failure by looking at the return value.  Callers have to call
> getfsuid() and getfsgid() to check that the IDs where actually
> changed.

Huh? If the documentation is correct, checking for error is easy:

    if (setfsuid(uid)!=uid) /* error */

The possibility that you could check for failure with a subsequent get
call rather than using the return value applies to all of the
set*[ug]id functions, not just these, so if it's an argument to remove
__wur from these then it's an argument to remove __wur from them all,
which I would actually agree with. IMO, __wur should only be used for
functions where it's impossible for the program to be correct if the
return value is not used, not for functions where it's just the
standard idiom but there are other equally-valid ways to check whether
the operation requested was performed successfully.

Rich


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]