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. > > 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, > > That puzzles me a bit... > > > 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 = newCwd.DirectoryHandle; > > > > Params.CurrentDirectoryName.Buffer = newCwd.Path.Buffer; > > ...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 == 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 >= 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 > > << " <== critical section" << endl; > > cout << showbase << hex << (size_t)Cwd > > << " <== Vista++ CWD struct pointer" << endl; > > 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
