Good idea! :) ill do that, as for why the PEB lock matters, in CL.exe there is a code that does the following: PWCHAR GetMergedEnv(PWCHAR MoreEnv) { ULONG Len; PWCHAR Env = GetEnviornmentStringW(); Len = GetEnvLength(Env) + GetEnvLength(MoreEnv); // while it traverses the Env in the PEB without a lock there is another thread which is doing SetEnvVar, which modifies the buffer it traverses
// this leads into the wrong Len being calulcated... and eventually the fatal error during Concat... PWCHAR NewEnv = malloc(Len); ConcatEnv(NewEnv, Env); ConcatEnv(NewEnv, MoreEnv); return NewEnv; }
I'll send a new patchset with a test tomorrow -- Jon.
On Sun, Mar 17, 2019 at 5:51 PM Nikolay Sivov nsivov@codeweavers.com wrote:
On 3/17/19 6:27 PM, Jon Doron wrote:
Also one thing worth noting how do you want me to code a test for this? This part in the PEB is opaque (https://docs.microsoft.com/en-us/windows/desktop/api/winternl/)
Just call it two times and compare pointers.
Thanks, Jon.
On Sun, Mar 17, 2019, 17:23 Jon Doron <arilou@gmail.com mailto:arilou@gmail.com> wrote:
Hi Nikolay I looked and noticed on XP the behavior is what's currently in Wine but from NT6 aka Vista it's the new behavior that I have implemented Do you want me to code a test that runs only if it's nt 6 and above?It should run on both, with one case marked broken() for example. I think the real question is why this has anything to do with PEB lock, if current implementation does not require locking.
Thanks, Jon. On Sat, Mar 16, 2019, 21:02 Nikolay Sivov <nsivov@codeweavers.com <mailto:nsivov@codeweavers.com>> wrote: On 3/16/19 9:57 PM, Jon Doron wrote: > There are certain applications which try to traverse the environement > being returned, but this is problematic since they cannot acquire the > PEB Lock (i.e cl.exe on Visual Studio 14.15) . To resolve the issue > provide a copy of the current environment same as in > GetEnvironmentStringsA . Please add a test to confirm this change.