[PATCH v2 0/1] MR74: ntdll: Prevent double free (Coverity)

[Fabian Maurer (＠DarkShadow44)]

The fd gets overwritten, therefore we need to close it before doing so.

Signed-off-by: Fabian Maurer 
---
Supersedes patch 233975

--
  v2: ntdll: Prevent double free (Coverity)

https://gitlab.winehq.org/wine/wine/-/merge_requests/74

[PATCH v2 1/1] ntdll: Prevent double free (Coverity)

[Fabian Maurer]

From: Fabian Maurer

The fd gets overwritten, therefore we need to close it before doing so.

Signed-off-by: Fabian Maurer 
---
Supersedes patch 233975
---
 dlls/ntdll/unix/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index 469c714f847..7e36909f3ac 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -4417,12 +4417,12 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
                 if (size > 0x10000) size = 0x10000;
                 if ((tmpbuf = malloc( size )))
                 {
+                    if (needs_close) close( fd );
                     if (!server_get_unix_fd( handle, FILE_READ_DATA, &fd, &needs_close, NULL, NULL ))
                     {
                         int res = recv( fd, tmpbuf, size, MSG_PEEK );
                         info->MessagesAvailable = (res > 0);
                         info->NextMessageSize = (res >= 0) ? res : MAILSLOT_NO_MESSAGE;
-                        if (needs_close) close( fd );
                     }
                     free( tmpbuf );
                 }

[PATCH v2 1/1] ntdll: Prevent double free (Coverity)

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=124967

Your paranoid android.

=== debian11 (build log) ===

Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24737.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24737.
Use of uninitialized value $Flaky in addition (+) at /home/testbot/lib/WineTestBot/LogUtils.pm line 720, <$LogFile> line 24737.

[Fabian Maurer (＠DarkShadow44)]

On Thu Oct 13 17:20:39 2022 +0000, Fabian Maurer wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/74/diffs?diff_id=13710&start_sha=cf7caf36c00dbbc8ed1d16b75a94106fdef78ca1#51bc191c8a1fc172f9ad299dd009ef2d24dfa043_4219_4426)

Yes, that makes sense, thanks. Either I missed that or it got lost in rebasing, can't tell after all this time...

[PATCH v2 0/1] MR74: ntdll: Prevent double free (Coverity) - approved

[Sven Baars (＠sbaars)]

This merge request was approved by Sven Baars.

[Sven Baars (＠sbaars)]

On Thu Oct 13 17:22:03 2022 +0000, Fabian Maurer wrote:
> Yes, that makes sense, thanks. Either I missed that or it got lost in
> rebasing, can't tell after all this time...

And I think in that case you also want to set needs_close to false so it doesn't close fd twice if server_get_unix_fd fails.

[Fabian Maurer (＠DarkShadow44)]

On Sat Oct 15 00:09:26 2022 +0000, Sven Baars wrote:
> And I think in that case you also want to set needs_close to false so it
> doesn't close fd twice if server_get_unix_fd fails.

`server_get_unix_fd` always sets needs_close to false at the beginning,can't we rely on that?

[PATCH v2 0/1] MR74: ntdll: Prevent double free (Coverity) - approved

[Sven Baars (＠sbaars)]

This merge request was approved by Sven Baars.

[Sven Baars (＠sbaars)]

On Sat Oct 15 00:19:50 2022 +0000, Fabian Maurer wrote:
> `server_get_unix_fd` always sets needs_close to false at the
> beginning,can't we rely on that?

Right, that works. Sorry!