Re: [PATCH v2] Cygwin: Ensure unused fd available for open()

Jon Turney <[email protected]> Mon, 1 Jun 2026 14:30:48 +0100
Newsgroups gmane.os.cygwin.patches
Message-ID <[email protected]>
On 28/05/2026 06:42, Mark Geisert wrote:
> The existing logic for open() assumes an fd is always available in
> the fdtable for a created file.  This leads to a situation where, if
> there is no fd available due to the OPEN_MAX limit being hit, the
> file is created but cannot be referenced by a Cygwin fd.
> 
> Move the fd reservation code to an earlier location within open().

Hmm... the more I stare at cygheap_fdnew, the less sure I am I 
understand what's going on.

I'm sure you considered this, but just so I can tell myself I've done 
due diligence, perhaps you can briefly explain why this doesn't create 
the opposite leak? (i.e. the reserved fd is released if actually opening 
the file fails).

> Reported-by: Christian Franke <[email protected]>
> Addresses: https://cygwin.com/pipermail/cygwin/2026-May/259664.html
> Signed-off-by: Mark Geisert <[email protected]>
> Fixes: e859706578ba (* autoload.cc (NtCreateFile): Add.)
> 
> ---
>   winsup/cygwin/syscalls.cc | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 7a8e5d4fd..2bea79768 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -1547,6 +1547,13 @@ open (const char *unix_path, int flags, ...)
>   	  fh = fh_file;
>   	}
>   
> +      /* Reserve an fdtable entry here, before calling open_with_arch() below.
> +         Otherwise there's a tiny chance of hitting OPEN_MAX further on which
> +         could create a new file without any way for Cygwin to refer to it. */
> +      cygheap_fdnew fd;
> +      if (fd < 0)
> +        __leave;		/* errno already set */
> +
>         if (fh->dev () == FH_PROCESSFD && fh->pc.follow_fd_symlink ())
>   	{
>   	  /* Reopen file by descriptor */
> @@ -1573,14 +1580,6 @@ open (const char *unix_path, int flags, ...)
>   	try_to_bin (fh->pc, fh->get_handle (), DELETE,
>   		    FILE_OPEN_FOR_BACKUP_INTENT);
>   
> -      cygheap_fdnew fd;
> -
> -      if (fd < 0)
> -	{
> -	  fh->close();
> -	  __leave;		/* errno already set */
> -	}
> -
>         fd = fh;
>         if (fd <= 2)
>   	set_std_handle (fd);