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

[Fabian Maurer]

fd will be closed at the end of the function anyways

Signed-off-by: Fabian Maurer 
---
 dlls/ntdll/unix/file.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
index cc8bf0c6e82..a6d76a49b27 100644
--- a/dlls/ntdll/unix/file.c
+++ b/dlls/ntdll/unix/file.c
@@ -4215,7 +4215,6 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
                         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 );
                 }
--
2.36.0

[PATCH v2 2/5] shell32: Prevent user after free in error case (Coverity)

[Fabian Maurer]

Otherwise when hr is not SUCCEEDED we use array and free it again.

Signed-off-by: Fabian Maurer 
---
 dlls/shell32/shellitem.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c
index 0a3a76cbd6a..8e66c6ab31a 100644
--- a/dlls/shell32/shellitem.c
+++ b/dlls/shell32/shellitem.c
@@ -1402,15 +1402,15 @@ HRESULT WINAPI SHCreateShellItemArrayFromIDLists(UINT cidl,
     if(SUCCEEDED(ret))
     {
         ret = create_shellitemarray(array, cidl, psia);
-        heap_free(array);
-        if(SUCCEEDED(ret))
-            return ret;
     }

-    for(i = 0; i < cidl; i++)
-        if(array[i]) IShellItem_Release(array[i]);
+    if(FAILED(ret))
+    {
+        for(i = 0; i < cidl; i++)
+            if(array[i]) IShellItem_Release(array[i]);
+        *psia = NULL;
+    }
     heap_free(array);
-    *psia = NULL;
     return ret;
 }

--
2.36.0

[PATCH v2 2/5] shell32: Prevent user after free in error case (Coverity)

[Fabian Maurer]

Hello Nikolay,

this is what you meant, correct?

Regards,
Fabian Maurer

On Dienstag, 26. April 2022 21:03:19 CEST you wrote:
> Otherwise when hr is not SUCCEEDED we use array and free it again.
> Signed-off-by: Fabian Maurer
>
> dlls/shell32/shellitem.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c
> index 0a3a76cbd6a..8e66c6ab31a 100644
> --- a/dlls/shell32/shellitem.c
> +++ b/dlls/shell32/shellitem.c
> @@ -1402,15 +1402,15 @@ HRESULT WINAPI
> SHCreateShellItemArrayFromIDLists(UINT cidl, if(SUCCEEDED(ret))
>      {
>          ret = create_shellitemarray(array, cidl, psia);
> -
>    heap_free(array);
> -
>    if(SUCCEEDED(ret))
> -
>        return ret;
>
> }
> -
>
> for(i = 0; i < cidl; i++)
> -
>    if(array[i]) IShellItem_Release(array[i]);
> - if(FAILED(ret))
> - {
> -
>    for(i = 0; i < cidl; i++)
> -
>        if(array[i]) IShellItem_Release(array[i]);
> -
>    *psia = NULL;
> - }
> heap_free(array);
> - *psia = NULL;
> return ret;
>
> }
>
> --
> 2.36.0

[PATCH v2 2/5] shell32: Prevent user after free in error case (Coverity)

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov

[PATCH v2 3/5] shell32: Refactor to keep style consistent

[Fabian Maurer]

Uses the same concept from the last commit.

Signed-off-by: Fabian Maurer 
---
 dlls/shell32/shellitem.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c
index 8e66c6ab31a..e6620c41e50 100644
--- a/dlls/shell32/shellitem.c
+++ b/dlls/shell32/shellitem.c
@@ -1286,16 +1286,13 @@ HRESULT WINAPI SHCreateShellItemArray(PCIDLIST_ABSOLUTE pidlParent,
     if(SUCCEEDED(ret))
     {
         ret = create_shellitemarray(array, cidl, ppsiItemArray);
-        if(SUCCEEDED(ret))
-        {
-            heap_free(array);
-            return ret;
-        }
     }

-    /* Something failed, clean up. */
-    for(i = 0; i < cidl; i++)
-        if(array[i]) IShellItem_Release(array[i]);
+    if(FAILED(ret))
+    {
+        for(i = 0; i < cidl; i++)
+            if(array[i]) IShellItem_Release(array[i]);
+    }
     heap_free(array);
     return ret;
 }
--
2.36.0

[PATCH v2 3/5] shell32: Refactor to keep style consistent

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov

[PATCH v2 4/5] mfplay: Prevent usage of uninitialized pointer in error case (Coverity)

[Fabian Maurer]

Later we check if they are set before calling IMFTopologyNode_Release,
but they are possibly never initialized

Signed-off-by: Fabian Maurer 
---
 dlls/mfplay/player.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mfplay/player.c b/dlls/mfplay/player.c
index 7d9a4da8649..e68c02a7e5c 100644
--- a/dlls/mfplay/player.c
+++ b/dlls/mfplay/player.c
@@ -1378,7 +1378,7 @@ static HRESULT media_item_create_sink_node(IUnknown *sink, IMFTopologyNode **nod

static HRESULT media_item_create_topology(struct media_player *player, struct media_item *item, IMFTopology **out)
 {
-    IMFTopologyNode *src_node, *sink_node;
+    IMFTopologyNode *src_node = NULL, *sink_node = NULL;
     BOOL selected, video_added = FALSE;
     IMFStreamDescriptor *sd;
     IMFTopology *topology;
--
2.36.0

[PATCH v2 5/5] mfplat: Prevent use after free (Coverity)

[Fabian Maurer]

Signed-off-by: Fabian Maurer 
---
 dlls/mfplat/mediatype.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/mfplat/mediatype.c b/dlls/mfplat/mediatype.c
index 71d55e2d232..b36a44d1e1c 100644
--- a/dlls/mfplat/mediatype.c
+++ b/dlls/mfplat/mediatype.c
@@ -976,6 +976,7 @@ static const MFVIDEOFORMAT * WINAPI video_mediatype_GetVideoFormat(IMFVideoMedia
     TRACE("%p.\n", iface);

CoTaskMemFree(media_type->video_format);
+    media_type->video_format = NULL;
     if (FAILED(hr = MFCreateMFVideoFormatFromMFMediaType(&media_type->IMFMediaType_iface, &media_type->video_format, &size)))
         WARN("Failed to create format description, hr %#lx.\n", hr);

@@ -1376,6 +1377,7 @@ static const WAVEFORMATEX * WINAPI audio_mediatype_GetAudioFormat(IMFAudioMediaT
     TRACE("%p.\n", iface);

CoTaskMemFree(media_type->audio_format);
+    media_type->audio_format = NULL;
     if (FAILED(hr = MFCreateWaveFormatExFromMFMediaType(&media_type->IMFMediaType_iface, &media_type->audio_format,
             &size, MFWaveFormatExConvertFlag_Normal)))
     {
--
2.36.0

[PATCH v2 5/5] mfplat: Prevent use after free (Coverity)

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov