[PATCH v1] kernel32: make GetEnvironmentStringsW returns a copy of the environment

[Jon Doron]

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 at 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 at gmail.com
> > <mailto:arilou at 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 at codeweavers.com
> >     <mailto:nsivov at 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.
> >