This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: [PATCH v2] Cygwin: Fix the address of myself
- From: Ken Brown <kbrown at cornell dot edu>
- To: "cygwin-patches at cygwin dot com" <cygwin-patches at cygwin dot com>
- Cc: Jon Turney <jon dot turney at dronecode dot org dot uk>
- Date: Wed, 24 Jul 2019 19:11:10 +0000
- Subject: Re: [PATCH v2] Cygwin: Fix the address of myself
- Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=cornell.edu;dmarc=pass action=none header.from=cornell.edu;dkim=pass header.d=cornell.edu;arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=eehjAB+Ifi7Xq2Op4gFCCuUqH1dHGStZIWf1RHEKvqg=; b=DmH0PvxmFg8yGnNhmTdwzQtB22qCpuEQfl7HybMxzKoDkQZd0apKbKKTNsy56+wtL+Wan54BAsAwgtNma6IEuXw+zWB0Zcti6/QK2Q0DZji7kUssfrFlKsYz+gBqy6RIGlP+p3tZYOjnejCDDh1qYNsGY7KrJCP9Q0VAUHDDqApScpjN1iiG3AOS7T5FSaipvZNYgRT5j9CmlGxA9IH8b6+1/kfywaabNUMkxLrsMXyXEInVZA1slM9AL+0Qb4AcghRbEvlIuG0bMHdCbRXiM7L4tuWzm9GG4h6Iu9F3SpSEsSQSSTpj6P+A86UfdY5O8OwaiINcJ7Hfsh0TqbEObg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iJxCMJBAA/is6v83svvZ5v4iOeESNWB7K6IS56yim8W5CEcjcRiF+sjLZX9fNPELevBjlNZOwxC21ywwFPplRBWD8Vq9xFIFgazsTIPXc/6I3PsBJglogIdtMoK+/0FoWMgiyaBK58fx99wmBtnpmfcuo/1kFK4OsxWPL2NltLDpachK+nMo+GmKLrRgofDlD4NEDFXxy50uKm0EsYxdLxndsgOCKlHJWfat6S3kF7GONhUWN6ePXg2/k/YuLpJdBDlx1RV+IeSQ0vT0iOhMQt7wnzcBVt+jAi7MbY1f8er9jFnf5zklDG4hQDG/+S1XKrwCvS0PtADB9h4Lf3wgKQ==
- References: <20190724162524.5604-1-corinna-cygwin@cygwin.com> <20190724165447.28339-1-corinna-cygwin@cygwin.com>
On 7/24/2019 12:54 PM, Corinna Vinschen wrote:
> From: Corinna Vinschen <corinna@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 <corinna@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