Mail Archives: cygwin/2010/09/13/15:48:03
On Sep 11 03:30, Corinna Vinschen:
> On Sep 11 00:12, John Carey wrote:
> > The proof-of-concept code follows (and is also attached). It includes
> > an analysis of what is going on--to the extent that I know what is going
> > on, of course. Please let me know what you think.
>=20
> First of all, I'm thoroughly impressed. This looks like a lot of
> detective work and I'm also impressed by the amount of time you
> apparently put into this.
Thank you.
> I'm hopeful we can create something for Cygwin from this. I have just
> a few discussing points for now.
If it's useful, great. (It's up to you, of course,
to decide if the maintainance burden is justified.)
> > The PEB does NOT seem to point to any VistaCwd instances. Instead,
>=20
> That puzzles me a bit...
>=20
> > After creating the new VistaCwd instance; call it newCwd, the
> > SetCurrentDirectory () implementation modifies the PEB and Cwd
> > under a lock on CwdCS, as follows:
> >
> > Params.CurrentDirectoryHandle =3D newCwd.DirectoryHandle;
> >
> > Params.CurrentDirectoryName.Buffer =3D newCwd.Path.Buffer;
>=20
> ...because, at this point it *does* point to the newCWD, even if only
> indirectly. Let's name the VistaCwd structure Cwd points to "curCwd"
> for now. Since we have the address of curCwd.Path.Buffer in
> Params.CurrentDirectoryName.Buffer, we can infer the address of curCwd
> from here, right? It's start is just a constant offset from the Buffer
> address.
Excellent point. Even though the PEB does not contain
the definitive pointer to the current VistaCwd instance,
one can deduce its value, as you say. (One must also use
the fact that newCwd.Path.Buffer =3D=3D newCwd.Buffer.)
> Given that, wouldn't it be potentially possible to override the content
> of curCwd? The problem is probably (just as in my old Cygwin code up to
> 1.7.5) to make sure that this is thread safe. Probably we would still
> need CwdCS.
Unfortunately, it looks as if Win32 API functions guarantee to each
other that they will not modify the path in a VistaCwd whose reference
count is >=3D 2. (Though it looks like they may update OldDismountCount
under a CwdCS lock.)
Consider RtlGetCurrentDirectory_U(). It shares a (non-exported)
subroutine with other Win32 API functions that acquires a counted
reference to the current VistaCwd instance under the protection of
a lock on CwdCS. (This subroutine also does some kind of freshness
check against DismountCount.) But that subroutine releases its
lock before returning, and RtlGetCurrentDirectory_U() then uses the
pathname in the returned VistaCwd instance *without* holding a lock.
Specifically, it copies that pathname to a memory buffer passed in by
its own caller. Pathname copies are not atomic. Thus, unless there
is a bug in RtlGetCurrentDirectory_U(), it has some guarantee that
other threads will not modify the pathname in its VistaCwd instance,
though they are free to allocate a new VistaCwd instance and assign
its address to the global Cwd pointer (so long as they lock CwdCS).
Presumably the point of the Vista++ CWD mechanism is to reduce
contention on the CWD lock. Threads acquire that lock for very
short periods of time. They appear to avoid making system calls
or even allocating memory while holding it. Also, the CWD lock
is no longer the PEB lock, so CWD usage does not serialize with
other PEB-related activities.
Anyway, it looks to me as if overwriting the path in an existing
VistaCwd instance would confuse RtlGetCurrentDirectory_U() and
probably other Win32 API functions. (It may be that the same is
true for the handle member; I have not tried to find that out.)
Cygwin should probably allocate a new VistaCwd instance,
as does the Win32 SetCurrentDirectory() implementation.
> > cout << showbase << hex << (size_t)CwdCS
> > << " <=3D=3D critical section" << endl;
> > cout << showbase << hex << (size_t)Cwd
> > << " <=3D=3D Vista++ CWD struct pointer" << endl;
>=20
> Is there any connection between these two addresses, like, say, they
> are side by side?
Not that I have been able to find. The gap between Cwd and CwdCS
is always positive, but is large and varies among OS versions:
32-bit Vista: 428 bytes
32-bit 2008: 548 bytes
64-bit: Vista, 2008, and Windows 7: 8412 bytes
> Can't we find Cwd by scanning the .data segment of ntdll.h for the
> address we inferred from the Params.CurrentDirectoryName.Buffer value?
Nice; that might work. But there would need to be some kind of rule
to pick a winner among multiple matching words, in case by coincidence
some other non-pointer word just happens to have the same bit pattern.
I can see two alternatives for the multiple-match case:
1. Code scanning, as suggested earlier. There would need to be a
unit test of the code scanner itself so that incompatible changes to
ntdll.dll could be detected deterministically, instead of only after
a multiple-match coincidence.
2. Call SetCurrentDirectory() and recheck the previously-matching
addresses to see which one matches the new value. Place some limit
(like 4) on the number of such retries, in case some new version of
ntdll.dll intentionally duplicates the pointer every time.
(Not sure what to do in that case; fall back to code scanning?)
> Thanks,
You're welcome. I hope this helps.
-- John
--
Problem reports: http://cygwin.com/problems.html
FAQ: http://cygwin.com/faq/
Documentation: http://cygwin.com/docs.html
Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
- Raw text -