Re: [PATCH 2/3] Implement mountee startup.

[Carl Fredrik Hammar]

Hi,

On Sun, Nov 22, 2009 at 09:05:16PM +0100, olafBuddenhagen@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.

Also credentials can be changed at any moment by other processes through
the msg_set_init_port() RPC (very much like a signal), which becomes
a problem if the number of UIDs grows between the calls to geteuid().
So it would be more proper with a loop, e.g.:

  nuids = geteuids (0, 0);
  do {
    old_nuids = nuids;
    uids = alloca (nuids * sizeof (uid_t));
    nuids = geteuids (nuids, uids);
  } while (old_nuids < nuids);

But alloca() should be replaced with realloc() and cleanup code for the
memory should be added.

Actually, I don't see how any use of geteuids() won't run into this
problem.  It would be much easier if it just returned malloced memory
to begin with...

Regards,
  Fredrik