This is the mail archive of the
cygwin-patches@cygwin.com
mailing list for the Cygwin project.
Re: setuid on Win95 and etc_changed, passwd & group.
Christopher Faylor wrote:
> >During testing I noticed another issue. If etc_changed is initialized
> >in a parent and /etc/passwd is changed between the moments where a
> >child is forked and where etc_changed is first called in the child,
> >etc_changed unexpectedly returns false in the child (WinME).
> >Not sure how to fix that, short of always rereading the files in
> >the child (when/if actually accessed). That would be an OK solution if
> >we hadn't just copied the data from the parent. Would it be possible
> >to store passwd and group in some other heap (from Windows?) that
> >doesn't get copied? If that was done, then the etc_changed handle
> >could be opened as needed instead of being inherited.
>
> It may be worse than what you say. It's possible that there is a huge
> race here where the parent and child compete for the privilege of each
> being able to figure out when /etc changed. I think that if the
> parent figures out that etc changed, the child will not notice and
> vice versa. This is regardless of whether the process is in the
> middle of forking or not.
>
> So, yes, you can easily put this in some other structure. Just
> move it out of cygheap entirely and use a global variable that
> is marked NO_COPY. Don't do the DuplicateHandle step to mark the
> file as inheritable. Then parent and child should both be able
> to figure out when etc changes.
Yes, this is what I will do.
The passwd state and the non-inheritable etc_handle will be NO_COPY.
The handle will be created as needed on first access in a process.
In addition forked processes, which may already have a malloced copy
of passwd, will do a date comparison on first access.
> >Incidentally while looking at cygheap.cc I noticed that the
> >+ sizeof (_cmalloc_entry) on line 221 duplicates the one on line
> >234. I didn't change it in this patch as it is not related to the rest,
> >but I have run with an abbreviated line 221 for a day.
>
> I'm not sure what you're saying. Are you saying it's inefficient
> because it is duplicated? I don't see anything wrong with the code.
I believe there are 8 unused bytes in every block.
On line 221 sz is what's asked + 8.
On line 234 size is sz + 8, or what's asked + 16.
The header has size 8, the last 8 bytes will never be filled.
> In a similar vein,
>
> BOOL isuninitialized () const
> {
> if (state == uninitialized)
> (void) cygheap->etc_changed (me);
> return (state == uninitialized);
> }
>
> Are both tests for uninitialized necessary? If not shouldn't it be
> something like:
>
> BOOL isuninitialized () const
> {
> if (state != uninitialized)
> return false;
> (void) cygheap->etc_changed (me);
> return true;
> }
I like functions with a single return, within reason.
I thought the compiler would be smart enough not to
test twice.
> Also, could you explain what this 'me' stuff is wrt etc_changed?
That's to go around the problem outlined in the e-mail. Objects
accessing etc_changed (for now passwd and group) have an ID (me).
When an object discovers that etc has changed, it sets a flag for
all *other* objects, telling them that etc has already changed
(see new code in cygheap.cc).
Pierre