June 2022 - Wine-Devel - list.winehq.org

[PATCH v2 4/9] user32/tests: Workaround use after free warnings.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/clipboard.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/user32/tests/clipboard.c b/dlls/user32/tests/clipboard.c index e05de29805e..3131c2e0e8c 100644 --- a/dlls/user32/tests/clipboard.c +++ b/dlls/user32/tests/clipboard.c @@ -29,6 +29,7 @@ static BOOL (WINAPI *pAddClipboardFormatListener)(HWND hwnd); static BOOL (WINAPI *pRemoveClipboardFormatListener)(HWND hwnd); static BOOL (WINAPI *pGetUpdatedClipboardFormats)( UINT *formats, UINT count, UINT *out_count ); +static HGLOBAL (WINAPI *pGlobalFree)(HGLOBAL); static int thread_from_line; static char *argv0; @@ -2073,7 +2074,7 @@ static void test_data_handles(void) metafile = create_metafile(); h = SetClipboardData( CF_METAFILEPICT, metafile ); ok( h == metafile, "Expected metafilepict %p, got %p.\n", metafile, h ); - ok( !GlobalFree( metafile ), "GlobalFree failed.\n" ); + ok( !pGlobalFree( metafile ), "GlobalFree failed.\n" ); h = GetClipboardData( CF_METAFILEPICT ); ok( h == metafile, "Expected metafile %p, got %p.\n", metafile, h ); ok( is_freed( h ), "Expected freed mem %p.\n", h ); @@ -2081,7 +2082,7 @@ static void test_data_handles(void) metafile = create_metafile(); h = SetClipboardData( CF_DSPMETAFILEPICT, metafile ); ok( h == metafile, "Expected metafilepict %p, got %p.\n", metafile, h ); - ok( !GlobalFree( metafile ), "GlobalFree failed.\n" ); + ok( !pGlobalFree( metafile ), "GlobalFree failed.\n" ); h = GetClipboardData( CF_DSPMETAFILEPICT ); ok( h == metafile, "Expected metafile %p, got %p.\n", metafile, h ); ok( is_freed( h ), "Expected freed mem %p.\n", h ); @@ -2343,6 +2344,7 @@ START_TEST(clipboard) pAddClipboardFormatListener = (void *)GetProcAddress( mod, "AddClipboardFormatListener" ); pRemoveClipboardFormatListener = (void *)GetProcAddress( mod, "RemoveClipboardFormatListener" ); pGetUpdatedClipboardFormats = (void *)GetProcAddress( mod, "GetUpdatedClipboardFormats" ); + pGlobalFree = (void *)GetProcAddress( GetModuleHandleA( "kernel32" ), "GlobalFree" ); if (argc == 4 && !strcmp( argv[2], "set_clipboard_data" )) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/180

June 6, 2022

[PATCH v2 3/9] oleaut32/tests: Workaround use after free warnings.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/oleaut32/tests/vartype.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dlls/oleaut32/tests/vartype.c b/dlls/oleaut32/tests/vartype.c index c5137572d9d..d955a6d86da 100644 --- a/dlls/oleaut32/tests/vartype.c +++ b/dlls/oleaut32/tests/vartype.c @@ -31,6 +31,8 @@ DEFINE_GUID(UUID_test_struct, 0x4029f190, 0xca4a, 0x4611, 0xae,0xb9,0x67,0x39,0x83,0xcb,0x96,0xdd); +static void (WINAPI *pSysFreeString)(BSTR); + /* Some Visual C++ versions choke on __uint64 to float conversions. * To fix this you need either VC++ 6.0 plus the processor pack * or Visual C++ >=7.0. @@ -5843,9 +5845,9 @@ static void test_bstr_cache(void) str = SysAllocString(testW); /* This should put the string into cache */ - SysFreeString(str); + pSysFreeString(str); /* The string is in cache, this won't touch it */ - SysFreeString(str); + pSysFreeString(str); ok(SysStringLen(str) == 4, "unexpected len\n"); ok(!lstrcmpW(str, testW), "string changed\n"); @@ -5874,8 +5876,8 @@ static void test_bstr_cache(void) str2 = SysAllocStringLen(NULL, 16); ok(str2 == strs[1], "str2 != strs[1]\n"); - SysFreeString(str); - SysFreeString(str2); + pSysFreeString(str); + pSysFreeString(str2); SysFreeString(str); SysFreeString(str2); } @@ -6047,6 +6049,7 @@ static void test_recinfo(void) START_TEST(vartype) { hOleaut32 = GetModuleHandleA("oleaut32.dll"); + pSysFreeString = (void *)GetProcAddress(hOleaut32, "SysFreeString"); has_i8 = GetProcAddress(hOleaut32, "VarI8FromI1") != NULL; has_locales = has_i8 && GetProcAddress(hOleaut32, "GetVarConversionLocaleSetting") != NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/180

June 6, 2022

[PATCH v2 2/9] mshtml/tests: Avoid using pointer value after free.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mshtml/tests/misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mshtml/tests/misc.c b/dlls/mshtml/tests/misc.c index 410d24884ee..72692c4915b 100644 --- a/dlls/mshtml/tests/misc.c +++ b/dlls/mshtml/tests/misc.c @@ -240,13 +240,13 @@ static void test_HTMLStorage(void) hres = IHTMLStorage_getItem(storage, key, &var); ok(hres == S_OK, "getItem failed: %08lx\n", hres); ok(V_VT(&var) == VT_NULL, "got %d\n", V_VT(&var)); - SysFreeString(key); hres = IHTMLStorage_getItem(storage, NULL, NULL); ok(hres == E_POINTER, "getItem returned: %08lx\n", hres); hres = IHTMLStorage_getItem(storage, key, NULL); ok(hres == E_POINTER, "getItem failed: %08lx\n", hres); + SysFreeString(key); V_VT(&var) = 0xdead; hres = IHTMLStorage_getItem(storage, NULL, &var); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/180

June 6, 2022

[PATCH v2 1/9] mscoree: Avoid using pointer after free.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/mscoree/corruntimehost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mscoree/corruntimehost.c b/dlls/mscoree/corruntimehost.c index 1651ca69d29..42081ad99bb 100644 --- a/dlls/mscoree/corruntimehost.c +++ b/dlls/mscoree/corruntimehost.c @@ -1917,7 +1917,7 @@ HRESULT create_monodata(REFCLSID clsid, LPVOID *ppObj) HeapFree(GetProcessHeap(), 0, filenameA); if (!assembly) { - ERR("Cannot open assembly %s, status=%i\n", filenameA, status); + ERR("Cannot open assembly %s, status=%i\n", debugstr_w(filename), status); domain_restore(prev_domain); goto cleanup; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/180

June 6, 2022

[PATCH v2 0/9] MR180: misc: Fixes and workarounds for UAF warnings. (GCC12)

by Rémi Bernon

The mscoree one is a genuine UAF bug, other are more arguably false positive but GCC doesn't like freed pointer value to be used in any way. -- v2: server: Avoid using pointer value after realloc. d3drm: Avoid using pointer value after free. dsound: Avoid using pointer value after free. notepad: Avoid using pointer value after free. msi: Avoid using pointer value after free. user32/tests: Workaround use after free warnings. oleaut32/tests: Workaround use after free warnings. mshtml/tests: Avoid using pointer value after free. mscoree: Avoid using pointer after free. https://gitlab.winehq.org/wine/wine/-/merge_requests/180

June 6, 2022

Re: [PATCH 0/6] MR179: winegstreamer: Use the wg_sample structure to wrap and read media samples.

by Zebediah Figura

I've been staring at this for a while, and I'm still not convinced that this abstraction is an improvement. I think I get the idea—to try to share code between the input and output sample path—but as far as I can tell the only code that's actually *shared* is the code to retrieve the buffer pointer itself and the maximum length. Everything else is either input-only or output-only. As such we end up introducing a lot of code into the common path that kind of obscures the otherwise simpler functionality of push/read. (It's also kind of confusing that we're reading the previous values of an output sample, or writing values into an input sample.) I don't intend to let that block progress on the transform objects, but if it's possible I'd rather hold off on converting the parser to this abstraction—at least until the transforms have reached their "final" (zero-copy) form [not that it's immediately clear why this abstraction makes more sense in that case]. Would it be feasible to hold off on these patches for now?

June 6, 2022

[tools] winetest: Apply a couple of spelling fixes.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- winetest/build-patterns | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/winetest/build-patterns b/winetest/build-patterns index 0bad35120..0bab17180 100755 --- a/winetest/build-patterns +++ b/winetest/build-patterns @@ -1477,7 +1477,7 @@ EOF <li>Related bugs should have the source and testcase keywords, and their description should contain module:unit, module/unit or module/tests/unit. </ul> -The patterns are grouped according to the likelyhood that they contain new failures: +The patterns are grouped according to the likelihood that they contain new failures: <ul>"; my @listids = ("summary", "recent", "newmode", "regular", "old", @@ -1499,7 +1499,7 @@ EOF $tests{0}->{desc} .= "</ul>"; if (!$nolist) { - my @table = ("<details><summary>Page cutomization</summary> + my @table = ("<details><summary>Page customization</summary> <div class='detailsbox'>This section lets you select the reports to show on this page. <label><input id='reruns' type='checkbox' checked> Hide WineTest reruns</label> "); if (!$subpage or $subpage eq "-tb-win") -- 2.30.2

June 6, 2022

[tools] testbot/cgi: Tweak the CollectionBlock master columns iterations.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/ObjectModel/CGI/CollectionBlock.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm index 336c7cd05..e8156a1bf 100644 --- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm +++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm @@ -243,7 +243,7 @@ sub GetDetailsLink($$) my ($MasterColNames, $MasterColValues) = $Row->{Item}->GetMasterCols(); if (defined $MasterColNames) { - foreach my $ColIndex (0 .. @$MasterColNames - 1) + foreach my $ColIndex (0..$#{$MasterColNames}) { $Row->{DetailsLink} .= "&". $MasterColNames->[$ColIndex] ."=". uri_escape($MasterColValues->[$ColIndex]); @@ -385,7 +385,7 @@ sub GenerateFormStart($) my ($MasterColNames, $MasterColValues) = $self->{Collection}->GetMasterCols(); if (defined($MasterColNames)) { - foreach my $ColIndex (0..scalar @$MasterColNames - 1) + foreach my $ColIndex (0..$#{$MasterColNames}) { print "<div><input type='hidden' name='", $MasterColNames->[$ColIndex], "' value='", $self->escapeHTML($MasterColValues->[$ColIndex]), @@ -684,7 +684,7 @@ sub OnAction($$) my ($MasterColNames, $MasterColValues) = $self->{Collection}->GetMasterCols(); if (defined($MasterColNames)) { - foreach my $ColIndex (0 .. @$MasterColNames - 1) + foreach my $ColIndex (0..$#{$MasterColNames}) { $Target .= ($ColIndex == 0 ? "?" : "&") . $MasterColNames->[$ColIndex] . "=" . uri_escape($MasterColValues->[$ColIndex]); -- 2.30.2

June 6, 2022

[tools] testbot: Simplify building a slash-separated Item identifiers.

by Francois Gouget

This also fixes the task id in the GetReportNames() error messages. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/bin/Janitor.pl | 2 +- testbot/lib/ObjectModel/Item.pm | 19 +++++++++++++++++++ testbot/lib/WineTestBot/Steps.pm | 3 +-- testbot/lib/WineTestBot/Tasks.pm | 13 +++++-------- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/testbot/bin/Janitor.pl b/testbot/bin/Janitor.pl index ab72ac1b6..af1123aed 100755 --- a/testbot/bin/Janitor.pl +++ b/testbot/bin/Janitor.pl @@ -205,7 +205,7 @@ if (%DeletedVMs) { if (exists $DeletedVMs{$Task->VM->Name}) { - Trace "Keeping the ", $Task->VM->Name, " VM for task ", join("/", @{$Task->GetMasterKey()}), "\n"; + Trace "Keeping the ", $Task->VM->Name, " VM for task ", $Task->GetSlashKey(), "\n"; delete $DeletedVMs{$Task->VM->Name}; last if (!%DeletedVMs); } diff --git a/testbot/lib/ObjectModel/Item.pm b/testbot/lib/ObjectModel/Item.pm index 0b1da92a2..39c80cc89 100644 --- a/testbot/lib/ObjectModel/Item.pm +++ b/testbot/lib/ObjectModel/Item.pm @@ -350,6 +350,25 @@ sub GetMasterKey($) return (\@MasterColNames, \@MasterColValues); } +=pod +=over 12 + +=item C<GetSlashKey()> + +Returns a string containing all the slash-separated key components. + +This can be used to display the key in the GUI or to build file paths (assuming +the key components can be trusted). + +=back +=cut + +sub GetSlashKey($) +{ + my ($self) = @_; + return join("/", @{$self->GetMasterKey()}); # drops the column names +} + sub ResetModified($) { my ($self) = @_; diff --git a/testbot/lib/WineTestBot/Steps.pm b/testbot/lib/WineTestBot/Steps.pm index 860dc4920..cd585e9db 100644 --- a/testbot/lib/WineTestBot/Steps.pm +++ b/testbot/lib/WineTestBot/Steps.pm @@ -128,8 +128,7 @@ sub Validate($) sub GetDir($) { my ($self) = @_; - my ($JobId, $StepNo) = @{$self->GetMasterKey()}; - return "$DataDir/jobs/$JobId/$StepNo"; + return "$DataDir/jobs/". $self->GetSlashKey(); } sub CreateDir($) diff --git a/testbot/lib/WineTestBot/Tasks.pm b/testbot/lib/WineTestBot/Tasks.pm index 50ce9604a..8c9f89ce8 100644 --- a/testbot/lib/WineTestBot/Tasks.pm +++ b/testbot/lib/WineTestBot/Tasks.pm @@ -91,8 +91,7 @@ sub InitializeNew($$) sub GetDir($) { my ($self) = @_; - my ($JobId, $StepNo, $TaskNo) = @{$self->GetMasterKey()}; - return "$DataDir/jobs/$JobId/$StepNo/$TaskNo"; + return "$DataDir/jobs/". $self->GetSlashKey(); } sub CreateDir($) @@ -118,13 +117,11 @@ sub GetReportNames($) return ($ErrMessage, undef, undef) if (defined $ErrMessage); if (!@$Missions) { - my @TaskKey = $self->GetMasterKey(); - return ("Task @TaskKey has no mission", undef, undef); + return ("Task ". $self->GetSlashKey() ." has no mission", undef, undef); } if (@$Missions > 1) { - my @TaskKey = $self->GetMasterKey(); - return ("Task @TaskKey should not have missions for multiple tasks\n", undef); + return ("Task ". $self->GetSlashKey() ." should not have missions for multiple tasks\n", undef); } my $TaskMissions = $Missions->[0]; @@ -277,7 +274,6 @@ sub UpdateStatus($$) if ($Status eq "running" and ($VM->Status ne "running" or !$VM->HasRunningChild())) { - my ($JobId, $StepNo, $TaskNo) = @{$self->GetMasterKey()}; my $OldUMask = umask(002); my $TaskDir = $self->CreateDir(); if (open TASKLOG, ">>", "$TaskDir/testbot.log") @@ -288,7 +284,8 @@ sub UpdateStatus($$) umask($OldUMask); require WineTestBot::Log; - WineTestBot::Log::LogMsg("Child process for task $JobId/$StepNo/$TaskNo died unexpectedly\n"); + WineTestBot::Log::LogMsg("Child process for task ", $self->GetSlashKey(), + " died unexpectedly\n"); # A crash probably indicates a bug in the task script but getting stuck # could happen due to network issues. So requeue the task like its script -- 2.30.2

June 6, 2022

[tools] testbot/orm: Simplify CombineKey() and SplitKey() a bit.

by Francois Gouget

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- testbot/lib/ObjectModel/Collection.pm | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/testbot/lib/ObjectModel/Collection.pm b/testbot/lib/ObjectModel/Collection.pm index 63d470bbc..1a48eec35 100644 --- a/testbot/lib/ObjectModel/Collection.pm +++ b/testbot/lib/ObjectModel/Collection.pm @@ -462,18 +462,14 @@ sub IsEmpty($) sub CombineKey($@) { - my $self = shift; - - my $CombinedKey = join("#@#", @_); - return $CombinedKey; + my $_self = shift; + return join("#@#", @_); } sub SplitKey($$) { - my ($self, $CombinedKey) = @_; - - my @KeyComponents = split /#@#/, $CombinedKey; - return @KeyComponents; + my ($_self, $CombinedKey) = @_; + return split /#@#/, $CombinedKey; } =pod -- 2.30.2

June 6, 2022