DMARC-Filter:	OpenDMARC Filter v1.4.2 delorie.com 4AILXK4W2824707
Authentication-Results:	delorie.com; dmarc=pass (p=none dis=none) header.from=cygwin.com
Authentication-Results:	delorie.com; spf=pass smtp.mailfrom=cygwin.com
DKIM-Filter:	OpenDKIM Filter v2.11.0 delorie.com 4AILXK4W2824707
Authentication-Results:	delorie.com;
	dkim=pass (1024-bit key, unprotected) header.d=cygwin.com header.i=@cygwin.com header.a=rsa-sha256 header.s=default header.b=E3MvRdsM
X-Recipient:	archive-cygwin AT delorie DOT com
DKIM-Filter:	OpenDKIM Filter v2.11.0 sourceware.org 99EC43858C32
DKIM-Signature:	v=1; a=rsa-sha256; c=relaxed/relaxed; d=cygwin.com;
	s=default; t=1731965598;
	bh=WNLzk97grNNX4KAhGZlLcPnk9aiFpGlp/EhtM+vLSAc=;
	h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe:
	List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:
	From;
	b=E3MvRdsM7LEZA4ak10ESg2Q4Hg4yQfoM5TAjW+RtHT+E5h000Ux4m/XaKA5lrQdbz
	nJ7JJezzWBB+36mtLMQ9OcFyGmT3nR3IDhZVZmaPQBrvQU0Olz2ZiCchU3tTG7N8Ns
	jnu2KZu5BLDHygQkAl1P4X8FgE12kNfjc+Fn1UFM=
X-Original-To:	cygwin AT cygwin DOT com
Delivered-To:	cygwin AT cygwin DOT com
DMARC-Filter:	OpenDMARC Filter v1.4.2 sourceware.org 8AFEE3858C33
ARC-Filter:	OpenARC Filter v1.0.0 sourceware.org 8AFEE3858C33
ARC-Seal:	i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1731965570; cv=none;
	b=oJB+mc+NWiiBGzVz/kV3R3MSSiDBtAH7LyVGyMOqptLXXH4FrOcsejwjyMkH4BXlz6kWKy1L2eRsbr3w6bc71rXcHc6or24F+b3fb47Dju3uMTKF2dGeLwt0gxuY1tIL0M21EGXDAsuS/zxXpSIOcElZFN9mGnkpfKxM22XP8r4=
ARC-Message-Signature:	i=1; a=rsa-sha256; d=sourceware.org; s=key;
	t=1731965570; c=relaxed/simple;
	bh=dw9FyETEoMdRpEgXitL9lu7ZakagZj9MfpEbVgRKtKo=;
	h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From;
	b=kc26YvOBe22quP5WgZIdFew49JH3VZ2SvMV8ljRgVb+mZZqql6OEVqWgMPY0LXMwf/Lhfx/YRiTPzk5QB4Ksw3uzC0nHO3m3VRAzYTL1IVYAefTOqmK3+8nN5yn+c0pnAZnAyQ2HH3Z/aJ9Q23YTaoGk9FLHx58NpR2pJ0YIE1E=
ARC-Authentication-Results:	i=1; server2.sourceware.org
Message-ID:	<309f90e4-d7e9-495c-955b-ee264ad74159@mailbox.org>
Date:	Mon, 18 Nov 2024 22:32:46 +0100
MIME-Version:	1.0
Subject:	Re: Possible issue with check_dir_not_empty
To:	cygwin AT cygwin DOT com
References:	<9f95d44b-2a46-4da8-9177-fc9b60a6d18e AT mailbox DOT org>
	<Zztwu6p77XC19HwJ AT calimero DOT vinschen DOT de>
Autocrypt:	addr=bernhardu AT mailbox DOT org; keydata=
	xsBNBFLFOk0BCADSnIOyw5hwhDXc5yKipb7asfzYmBwgRL9NfM9W8fNz8G5iphXTX2cAvrMy
	7CAOaktgz3yvWiJvf3l5GAB7FrA7HaY0sLRbhCcS06jvEAH3HScfaK+qPeJA1mHo11VudH5E
	oaOUKNz6xy5Gb30SuISwVF+i30GeJNi3DTk8zGjqPQHEQiEo8CJ0Hc1r/fUgo+F9f1ugpZNG
	Hn/ZJowpfCGRcNrrRC199MDI2m4q00i1bda8Z1P2PZTUZDWjD3dpzfkvVLSU+68Y49Kyv934
	Qkj6LV/qw+4wnexWtgBCBTklz0ocVAN2wh/6VQ6ny3I3pJd9Phyd5qDmURZ08u8BZXEtABEB
	AAHNK0Jlcm5oYXJkIMOcYmVsYWNrZXIgPGJlcm5oYXJkdUBtYWlsYm94Lm9yZz7CwJUEEwEK
	AD8CGyMGCwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAFiEE7HR0CmaRX6NuV4XV6W+iX4OmQeYF
	AmUarOEFCRf5nZMACgkQ6W+iX4OmQeaArAf/W+zJJHAQX60M9+P6RsZ/eIy+FLGCR1nJptSx
	x3LHDp7qQG1ZrMTyKBwyEcg9HGVWimg+KIdQaZaT3xnpbOGDz05GCDpN6gCiuCpEeLAHVnIp
	glIpHVfpVf3QMzvW+1Sy7bqSnxtXex62eY76SR/tmaGUUrLc5SwKu9xRMxzpT6ALkd76B9/L
	xzf2RyB2wgYEZN9r24vejivsdweBuSE/kZr4rNy1KkBIyosL0M0oOWthuZyy6rSyyJLyPiDV
	IfVgJ3zyqjWtznO4CGcSS3Uqq7YB+kp5KYcPRB373rq17vN/I/5mLV9W66pANzYjEyp4g9mX
	V/AFpX+d20zdrdjGYc7ATQRSxTpNAQgAzY7Lt75go8+7R9OrlxJgLvy2Btad9nI97eiQJ2So
	JW9VF8GpxdTHrHQSst1AuptxlxnsNJ60ajRA8GjQD62VrLwNrm/CJFmKVY1lnwQpk5eM7vdo
	f6zIEd1wLuuYr11tezWEjAky0J7X5sdz3ToPZh+HeIrQYEHexHJe+4vOjelW9nW0un+Hi5jk
	hjw2VwGp8lT+tq9fRxeqUL+L9ctW3btsz/Uf+1h/s3OGo/dlZSTFVAozeHYNZyiyu3Mt9WDs
	Ppbjgz7/FSxmWyworAw1yQeaQyBQgg3S3CH4euepWfBoa71EDH+I4Fw3/17d4wPiR8MXjjSD
	rlMwpV2dd6pj6wARAQABwsB8BBgBCgAmAhsMFiEE7HR0CmaRX6NuV4XV6W+iX4OmQeYFAmUa
	rOEFCRf5nZMACgkQ6W+iX4OmQeZtyQf8CEBfXTO5luF9N3ev6wrwY4sJhJnpoHdrK0s0Tdz9
	Nu0KPxXDhl5OVGWJWDeTZ41FyAtMLVmfBtTa4LNd0zFBvGP5JYoavNME34hOGJIih9mLl+Qu
	nNsVh1okNAYfS2VHOQuESGA9fsSWgli1H84pM1kpqL4vGlVSH+OPAK0H51e6/poQHDHFUq6s
	qmPCDAU0pSYmuuOg/V98WoZW3mtuG1MVLhgNznenctuF7oHx63OdjpCGA0Gv2p78tnPOP2ZZ
	VadUH1G35w/XzlNEPerCul4mdCB/gaDvO4S6Fw7ceBvPy0MutKiJ2BdjD0kW02G3sLLmy8jY
	Xun5X2+YmyYMtA==
In-Reply-To:	<Zztwu6p77XC19HwJ@calimero.vinschen.de>
X-MBO-RS-META:	4rapoj55c3ekrns65oz6u645pssmd7uj
X-MBO-RS-ID:	13abd92a19bdb0d7617
X-BeenThere:	cygwin AT cygwin DOT com
X-Mailman-Version:	2.1.30
List-Id:	General Cygwin discussions and problem reports <cygwin.cygwin.com>
List-Unsubscribe:	<https://cygwin.com/mailman/options/cygwin>,
	<mailto:cygwin-request AT cygwin DOT com?subject=unsubscribe>
List-Archive:	<https://cygwin.com/pipermail/cygwin/>
List-Post:	<mailto:cygwin AT cygwin DOT com>
List-Help:	<mailto:cygwin-request AT cygwin DOT com?subject=help>
List-Subscribe:	<https://cygwin.com/mailman/listinfo/cygwin>,
	<mailto:cygwin-request AT cygwin DOT com?subject=subscribe>
From:	=?utf-8?q?Bernhard_=C3=9Cbelacker_via_Cygwin?= <cygwin AT cygwin DOT com>
Reply-To:	=?UTF-8?Q?Bernhard_=C3=9Cbelacker?= <bernhardu AT mailbox DOT org>
Errors-To:	cygwin-bounces~archive-cygwin=delorie DOT com AT cygwin DOT com
Sender:	"Cygwin" <cygwin-bounces~archive-cygwin=delorie DOT com AT cygwin DOT com>
X-MIME-Autoconverted:	from base64 to 8bit by delorie.com id 4AILXK4W2824707

Hello Corinna,

Am 18.11.24 um 17:52 schrieb Corinna Vinschen:
> Hi Bernhard,
> 
> On Nov 16 23:36, Bernhard Ãœbelacker via Cygwin wrote:
>> Hello everyone,
>>
>> Is is about the buffer allocated in check_dir_not_empty.
>>
>> The pointer pfni gets allocated the buffer at the begin,
>> and is used in the NtQueryDirectoryFile call before the loops.
>> In the loop the pointer pfni is also used as iterator.
>> Therefore it holds no longer the initial buffer at the call
>> to NtQueryDirectoryFile in the while conditition at the bottom.
> 
> Good catch, thank you!

Forgot to mention the background. I actually hit this issue with running
Cygwin's git.exe below a modified Wine checking out the tag 3.5.3 of
newlib-cygwin. Unfortunately reproducing this issue still needs a few 
additional Wine patches to finish Cygwin installation.


>> Attached is a possible modification to always use the allocated buffer.
>>
>> Kind regards,
>> Bernhard
> 
> Thanks for the patch.
> 
> Would you be ok if I apply a simplified version under your authorship?
> 
> Rather than add a pfni_it(erator), use pfni as iterator and add a
> pfni_buf variable.  This is a much smaller patch, and is more in line
> with the usual variable naming in Cygwin.
> 
> I also added a release message text and a Fixes: line to the commit
> message.
> 
> Below is the tweaked patch.  If you're ok with this version, I'll push
> it.


That would be great. Thanks for maintaining Cygwin.

Kind regards,
Bernhard


> 
> Thanks,
> Corinna
> 
> 
>  From fbd8b9d769135d6410b423eb9d82b49be52523bb Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Bernhard=20=C3=9Cbelacker?= <bernhardu AT mailbox DOT org>
> Date: Sat, 16 Nov 2024 18:09:50 +0100
> Subject: [PATCH] Cygwin: check_dir_not_empty: Avoid leaving the allocated
>   buffer.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> The pointer pfni gets allocated the buffer at the begin,
> and is used in the NtQueryDirectoryFile call before the loops.
> In the loop the pointer pfni is also used as iterator.
> Therefore it holds no longer the initial buffer at the call
> to NtQueryDirectoryFile in the while conditition at the bottom.
> 
> Fixes: 28fa2a72f8106 ("* syscalls.cc (check_dir_not_empty): Check surplus directory entries")
> Signed-off-by: Bernhard Ãœbelacker <bernhardu AT mailbox DOT org>
> ---
>   winsup/cygwin/release/3.5.5 |  3 +++
>   winsup/cygwin/syscalls.cc   | 10 ++++++----
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/winsup/cygwin/release/3.5.5 b/winsup/cygwin/release/3.5.5
> index 2ca4572db7ed..3088f8682b6b 100644
> --- a/winsup/cygwin/release/3.5.5
> +++ b/winsup/cygwin/release/3.5.5
> @@ -33,3 +33,6 @@ Fixes:
>   
>   - Fix type of pthread_sigqueue() first parameter to match Linux.
>     Addresses: https://cygwin.com/pipermail/cygwin/2024-September/256439.html
> +
> +- Fix potential stack corruption in rmdir() in a border case.
> +  Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256774.html
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index df7d3a14efd4..433739cda6e0 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -617,9 +617,10 @@ check_dir_not_empty (HANDLE dir, path_conv &pc)
>     IO_STATUS_BLOCK io;
>     const ULONG bufsiz = 3 * sizeof (FILE_NAMES_INFORMATION)
>   		       + 3 * NAME_MAX * sizeof (WCHAR);
> -  PFILE_NAMES_INFORMATION pfni = (PFILE_NAMES_INFORMATION)
> -				 alloca (bufsiz);
> -  NTSTATUS status = NtQueryDirectoryFile (dir, NULL, NULL, 0, &io, pfni,
> +  PFILE_NAMES_INFORMATION pfni_buf = (PFILE_NAMES_INFORMATION)
> +				     alloca (bufsiz);
> +  PFILE_NAMES_INFORMATION pfni;
> +  NTSTATUS status = NtQueryDirectoryFile (dir, NULL, NULL, 0, &io, pfni_buf,
>   					  bufsiz, FileNamesInformation,
>   					  FALSE, NULL, TRUE);
>     if (!NT_SUCCESS (status))
> @@ -631,6 +632,7 @@ check_dir_not_empty (HANDLE dir, path_conv &pc)
>     int cnt = 1;
>     do
>       {
> +      pfni = pfni_buf;
>         while (pfni->NextEntryOffset)
>   	{
>   	  if (++cnt > 2)
> @@ -677,7 +679,7 @@ check_dir_not_empty (HANDLE dir, path_conv &pc)
>   	  pfni = (PFILE_NAMES_INFORMATION) ((caddr_t) pfni + pfni->NextEntryOffset);
>   	}
>       }
> -  while (NT_SUCCESS (NtQueryDirectoryFile (dir, NULL, NULL, 0, &io, pfni,
> +  while (NT_SUCCESS (NtQueryDirectoryFile (dir, NULL, NULL, 0, &io, pfni_buf,
>   					   bufsiz, FileNamesInformation,
>   					   FALSE, NULL, FALSE)));
>     return STATUS_SUCCESS;


-- 
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple

- Raw text -