[PATCH v2 0/1] MR74: ntdll: Prevent double free (Coverity)
The fd gets overwritten, therefore we need to close it before doing so. Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- Supersedes patch 233975 -- v2: ntdll: Prevent double free (Coverity) https://gitlab.winehq.org/wine/wine/-/merge_requests/74
From: Fabian Maurer <dark.shadow4(a)web.de> The fd gets overwritten, therefore we need to close it before doing so. Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> --- 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 ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/74
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.
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...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/74#note_10573
This merge request was approved by Sven Baars. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/74
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/74#note_10818
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?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/74#note_10819
This merge request was approved by Sven Baars. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/74
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!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/74#note_10834
participants (4)
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Marvin -
Sven Baars (＠sbaars)