On 7/24/2019 12:54 PM, Corinna Vinschen wrote:
> From: Corinna Vinschen <cori...@vinschen.de>
> 
> v2: rephrase commit message
> 
> Introducing an independent Cygwin PID introduced a regression:
> 
> The expectation is that the myself pinfo pointer always points to a
> specific address right in front of the loaded Cygwin DLL.
> 
> However, the independent Cygwin PID changes broke this.  To create
> myself at the right address requires to call init with h0 set to
> INVALID_HANDLE_VALUE or an existing address:
> 
> void
> pinfo::init (pid_t n, DWORD flag, HANDLE h0)
> {
>    [...]
>    if (!h0 || myself.h)
>      [...]
>    else
>      {
>        shloc = SH_MYSELF;
>        if (h0 == INVALID_HANDLE_VALUE)       <-- !!!
>          h0 = NULL;
>      }
> 
> The aforementioned commits changed that so h0 was always NULL, this way
> creating myself at an arbitrary address.
> 
> This patch makes sure to set the handle to INVALID_HANDLE_VALUE again
> when creating a new process, so init knows that myself has to be created
> in the right spot.  While at it, fix a potential uninitialized handle
> value in child_info_spawn::handle_spawn.
> 
> Fixes: b5e1003722cb ("Cygwin: processes: use dedicated Cygwin PID rather than 
> Windows PID")
> Fixes: 88605243a19b ("Cygwin: fix child getting another pid after spawnve")
> Signed-off-by: Corinna Vinschen <cori...@vinschen.de>
> ---
>   winsup/cygwin/dcrt0.cc | 2 +-
>   winsup/cygwin/pinfo.cc | 3 +--
>   2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
> index fb726a739ccf..86ab7256484c 100644
> --- a/winsup/cygwin/dcrt0.cc
> +++ b/winsup/cygwin/dcrt0.cc
> @@ -652,7 +652,7 @@ void
>   child_info_spawn::handle_spawn ()
>   {
>     extern void fixup_lockf_after_exec (bool);
> -  HANDLE h;
> +  HANDLE h = INVALID_HANDLE_VALUE;
>     if (!dynamically_loaded || get_parent_handle ())
>         {
>       cygheap_fixup_in_child (true);
> diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
> index cdbd8bd7eaf3..b67d660ae04d 100644
> --- a/winsup/cygwin/pinfo.cc
> +++ b/winsup/cygwin/pinfo.cc
> @@ -62,11 +62,10 @@ pinfo::thisproc (HANDLE h)
>       {
>         cygheap->pid = create_cygwin_pid ();
>         flags |= PID_NEW;
> +      h = INVALID_HANDLE_VALUE;
>       }
>     /* spawnve'd process got pid in parent, cygheap->pid has been set in
>        child_info_spawn::handle_spawn. */
> -  else if (h == INVALID_HANDLE_VALUE)
> -    h = NULL;
>   
>     init (cygheap->pid, flags, h);
>     procinfo->process_state |= PID_IN_USE;
> 

I'll be glad to take a close look at this as you asked.  But I'm not familiar 
with this part of the code, so it will take me a little time.

Ken

Reply via email to