On Sat, 16 Aug 2025, Takashi Yano wrote:
> The class child_info_spawn has two constructors: one without arguments
> and one with two arguments. The former does not initialize any members.
> Commit 1f836c5f7394 used the latter to ensure that the local ch_spawn
> (i.e., ch_spawn_local) would be properly initialized. However, this was
> insufficient - it initialized only the base child_info members, not the
> fields specific to child_info_spawn. This led to the issue reported in
> https://cygwin.com/pipermail/cygwin/2025-August/258660.html.
>
> This patch updates the former constructor to properly initialize member
> variable 'ev' which was referred without initialization, and switches
> ch_spawn_local to use it.
>
> Addresses: https://cygwin.com/pipermail/cygwin/2025-August/258660.html
> Fixes: 1f836c5f7394 ("Cygwin: spawn: Make system() thread-safe")
> Reported-by: Denis Excoffier <[email protected]>
> Reviewed-by: Jeremy Drake <[email protected]>
> Signed-off-by: Takashi Yano <[email protected]>
> ---
> winsup/cygwin/local_includes/child_info.h | 5 +++--
> winsup/cygwin/spawn.cc | 2 +-
> winsup/cygwin/syscalls.cc | 2 +-
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/winsup/cygwin/local_includes/child_info.h
> b/winsup/cygwin/local_includes/child_info.h
> index 2da62ffaa..b8707b9ec 100644
> --- a/winsup/cygwin/local_includes/child_info.h
> +++ b/winsup/cygwin/local_includes/child_info.h
> @@ -33,7 +33,7 @@ enum child_status
> #define EXEC_MAGIC_SIZE sizeof(child_info)
>
> /* Change this value if you get a message indicating that it is out-of-sync.
> */
> -#define CURR_CHILD_INFO_MAGIC 0xacbf4682U
> +#define CURR_CHILD_INFO_MAGIC 0x39f766b5U
>
> #include "pinfo.h"
> struct cchildren
> @@ -148,7 +148,8 @@ public:
> char filler[4];
>
> void cleanup ();
> - child_info_spawn () {};
> + child_info_spawn () :
> + child_info (sizeof *this, _CH_NADA, false), ev (NULL) {};
I noticed that moreinfo is checked/used in cleanup too, but it is set in
worker. It'd probably be safer to initialize it too though. Looking at
child_info, subproc_ready seems to not be initialized if
need_subproc_ready is false. It'd probably be subject to the same issue
as ev.
> child_info_spawn (child_info_types, bool);
> void record_children ();
> void reattach_children ();
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 680f0fefd..6cd97ec17 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -950,7 +950,7 @@ spawnve (int mode, const char *path, const char *const
> *argv,
> if (!envp)
> envp = empty_env;
>
> - child_info_spawn ch_spawn_local (_CH_NADA, false);
> + child_info_spawn ch_spawn_local;
> switch (_P_MODE (mode))
> {
> case _P_OVERLAY:
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 863f8f23c..83a54ca05 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -4535,7 +4535,7 @@ popen (const char *command, const char *in_type)
> fcntl (stdchild, F_SETFD, stdchild_state | FD_CLOEXEC);
>
> /* Start a shell process to run the given command without forking. */
> - child_info_spawn ch_spawn_local (_CH_NADA, false);
> + child_info_spawn ch_spawn_local;
> pid_t pid = ch_spawn_local.worker ("/bin/sh", argv, environ, _P_NOWAIT,
> __std[0], __std[1]);
>
>
