Mail Archives: cygwin/2024/11/18/11:54:50
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!
> 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.
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;
--
2.47.0
--
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 -