Hi, 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? > Also credentials can be changed at any moment by other processes > through the msg_set_init_port() RPC (very much like a signal), Yeah, I realized that after thinking more about it. On IRC I told Sergiu to return to the original code, doing the same check again on the second call. Too bad I didn't properly think it through in the first place, before even suggesting the change... > 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... > It would be much easier if it just returned malloced memory to begin > with... Agreed. -antrik-
