Hi,
On Sat, Nov 28, 2009 at 12:36:07AM +0100, olafbuddenha...@gmx.net wrote:
> On Wed, Nov 25, 2009 at 07:59:33PM +0100, Carl Fredrik Hammar wrote:
> > On Sun, Nov 22, 2009 at 09:05:16PM +0100, olafbuddenha...@gmx.net wrote:
> > > On Thu, Nov 19, 2009 at 10:28:37AM +0200, Sergiu Ivanov wrote:
>
> > > > + /* Fetch the effective UIDs of the unionfs process. */
> > > > + nuids = geteuids (0, 0);
> > > > + if (nuids < 0)
> > > > +return EPERM;
> > > > + uids = alloca (nuids * sizeof (uid_t));
> > > > +
> > > > + nuids = geteuids (nuids, uids);
> > > > + assert (nuids > 0);
> > >
> > > Hrmph, I didn't spot this before: I don't think the assert() is right --
> > > "nuids" (or "ngids") being exactly 0, is probably a perfectly valid
> > > case... And even if it is not, the test in the assert should be
> > > equivalent to the EPERM test above, to avoid confusion.
> >
> > geteuids() actual error (in errno) should be returned instead of EPERM.
>
> Does geteuids() actually set errno?
Yes, it calls __hurd_fail() which sets it.
> > which becomes a problem if the number of UIDs grows between the calls
> > to geteuid().
>
> Not sure this is really a problem. If the credentials change in the
> middle of things, we can't rely on the set being current anyways; so
> it's probably fine if it's truncated to the old size...
But then you are using credentials that are neither the old ones nor the
new ones. This could only cause confusion. Aborting with ``setauth not
supported'' (or some such) when (new_len > old_len) is better then this.
This seems appropriate since setauth is probably not handled right after
this setup anyway. The only way to do this currently is by using file
descriptors instead of ports for directories. I don't know the code
well enough to tell whether this is appropriate in this case...
There is also a _hurd_reauth_hook but it, and the macros used to
manipulate it, is private to glibc. It could probably be used but it'd
be really ugly.
Regards,
Fredrik
