[Bug 51790] New: MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

Bug ID: 51790
           Summary: MSI is no longer able to detect correct architecture
                    for a custom action DLL
           Product: Wine
           Version: 6.17
          Hardware: x86-64
                OS: Linux
            Status: NEW
          Keywords: regression
          Severity: normal
          Priority: P2
         Component: kernel32
          Assignee: [e-mail]
          Reporter: [e-mail]
   Regression SHA1: 02268cb290493c899b56996405ca7c65a8046e3c
      Distribution: ---

MSI relies on GetBinaryType() to return correct architecture for a custom
action DLL, however GetBinaryType() started to intentionally fail for DLL
files. This for instance leads to inability to run a 32-bit action from a
64-bit installer process.

This essentially breaks any installer in a WoW64 Wine prefix. This bug should
probably have 'major' severity set.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #1 from Dmitry Timoshkov  ---
(In reply to Dmitry Timoshkov from comment #0)
> MSI relies on GetBinaryType() to return correct architecture for a custom
> action DLL, however GetBinaryType() started to intentionally fail for DLL
> files. This for instance leads to inability to run a 32-bit action from a
> 64-bit installer process.
>
> This essentially breaks any installer in a WoW64 Wine prefix. This bug
> should probably have 'major' severity set.

Whist it's a pretty serious regression, there was no attempt to fix it yet.

In case it's not recognizable on the first sight, here is the relevant commit:

commit 02268cb290493c899b56996405ca7c65a8046e3c
Author: Alexandre Julliard 
Date:   Thu Jul 1 10:47:08 2021 +0200

kernel32: Fix the DLL check in GetBinaryTypeW().

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51375
    Signed-off-by: Alexandre Julliard

diff --git a/dlls/kernel32/module.c b/dlls/kernel32/module.c
index adf6b3c9a60..415f7f65714 100644
--- a/dlls/kernel32/module.c
+++ b/dlls/kernel32/module.c
@@ -190,7 +190,7 @@ BOOL WINAPI GetBinaryTypeW( LPCWSTR name, LPDWORD type )
             status = NtQuerySection( mapping, SectionImageInformation, &info,
sizeof(info), NULL );
             CloseHandle( mapping );
             if (status) return FALSE;
-            if (!(info.ImageCharacteristics & IMAGE_FILE_DLL)) return FALSE;
+            if (info.ImageCharacteristics & IMAGE_FILE_DLL) return FALSE;
             switch (info.Machine)
             {
             case IMAGE_FILE_MACHINE_I386:

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #2 from Dmitry Timoshkov  ---
And the affected MSI code broken by that commit:
https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/msi/custom.c#l747

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #3 from Hans Leidekker  ---
Created attachment 71245
  --> https://bugs.winehq.org/attachment.cgi?id=71245
patch

Does his patch help?

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #4 from Dmitry Timoshkov  ---
(In reply to Hans Leidekker from comment #3)
> Created attachment 71245 [details]
> patch
>
> Does his patch help?

Thanks Hans. I can't at the moment test the affected intaller, but the code
seems to include a reasonable subset of the GetBinaryTypeW implementation.
Should it also handle STATUS_INVALID_IMAGE_WIN32/STATUS_INVALID_IMAGE_WIN64
like GetBinaryTypeW does (however I don't see where those can be returned
from)?

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|kernel32                    |msi

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #5 from Hans Leidekker  ---
(In reply to Dmitry Timoshkov from comment #4)
> (In reply to Hans Leidekker from comment #3)
> > Created attachment 71245 [details]
> > patch
> >
> > Does his patch help?
>
> Thanks Hans. I can't at the moment test the affected intaller, but the code
> seems to include a reasonable subset of the GetBinaryTypeW implementation.
> Should it also handle STATUS_INVALID_IMAGE_WIN32/STATUS_INVALID_IMAGE_WIN64
> like GetBinaryTypeW does (however I don't see where those can be returned
> from)?

Good point. We could get STATUS_INVALID_IMAGE_WIN64 in case of an installer
trying to load a 64-bit DLL in a 32-bit prefix (see server/mapping.c). We
handle that case explicitly in dlls/msi/custom.c but I see it won't work if
get_binary_type() fails.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

Hans Leidekker  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71245|0                           |1
        is obsolete|                            |

--- Comment #6 from Hans Leidekker  ---
Created attachment 71246
  --> https://bugs.winehq.org/attachment.cgi?id=71246
patch

This version also handles STATUS_INVALID_IMAGE_WIN_64.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #7 from Dmitry Timoshkov  ---
(In reply to Hans Leidekker from comment #6)
> Created attachment 71246 [details]
> patch
>
> This version also handles STATUS_INVALID_IMAGE_WIN_64.

Hans, are you planning to submit it for inclusion before code freeze, or you
are waiting for the testing results from me? I think that you can send it, the
patch looks good to me.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #8 from Hans Leidekker  ---
(In reply to Dmitry Timoshkov from comment #7)
> (In reply to Hans Leidekker from comment #6)
> > Created attachment 71246 [details]
> > patch
> >
> > This version also handles STATUS_INVALID_IMAGE_WIN_64.
>
> Hans, are you planning to submit it for inclusion before code freeze, or you
> are waiting for the testing results from me? I think that you can send it,
> the patch looks good to me.

I was hoping you could test it or point to an installer that is affected. It's
a regression, so the fix could go in after code freeze too.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #9 from Dmitry Timoshkov  ---
(In reply to Hans Leidekker from comment #8)
> (In reply to Dmitry Timoshkov from comment #7)
> > (In reply to Hans Leidekker from comment #6)
> > > Created attachment 71246 [details]
> > > patch
> > >
> > > This version also handles STATUS_INVALID_IMAGE_WIN_64.
> >
> > Hans, are you planning to submit it for inclusion before code freeze, or you
> > are waiting for the testing results from me? I think that you can send it,
> > the patch looks good to me.
>
> I was hoping you could test it or point to an installer that is affected.
> It's a regression, so the fix could go in after code freeze too.

I've tested new version of the patch and can confirm that it does fix the
affected installer. Thanks Hans.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #10 from Hans Leidekker  ---
(In reply to Dmitry Timoshkov from comment #9)
> > I was hoping you could test it or point to an installer that is affected.
> > It's a regression, so the fix could go in after code freeze too.
>
> I've tested new version of the patch and can confirm that it does fix the
> affected installer. Thanks Hans.

Thanks for testing. I've submitted the patch.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |4aa858c9badfb35bcc3cdd38abf
                   |                            |d40dc5b06c696
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Dmitry Timoshkov  ---
Committed as 4aa858c9badfb35bcc3cdd38abfd40dc5b06c696.

Thanks Hans.

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

Josesk Volpe  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from Josesk Volpe  ---
I'm not sure, but i suspect this bug was affecting Samsung J2ME SDK
(https://appdb.winehq.org/objectManager.php?sClass=version&iId=40200)
installer. It was crashing with no message, and rarely would display:
"Error Number: 0x80040707
Description: DLL function call crashed: ISRT._WaitOnDialog

Setup will now terminate."

It was fixed on Wine 7.0-rc1

¿Should i add this bug on that application bug lists, to help archiving and
tracking this bug?

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #13 from Hans Leidekker  ---
(In reply to Josesk Volpe from comment #12)
> I'm not sure, but i suspect this bug was affecting Samsung J2ME SDK
> (https://appdb.winehq.org/objectManager.php?sClass=version&iId=40200)
> installer. It was crashing with no message, and rarely would display:
> "Error Number: 0x80040707
> Description: DLL function call crashed: ISRT._WaitOnDialog
>
> Setup will now terminate."
>
> It was fixed on Wine 7.0-rc1
>
> ¿Should i add this bug on that application bug lists, to help archiving and
> tracking this bug?

Does the bug reappear if you revert this commit?

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

--- Comment #14 from Josesk Volpe  ---
N(In reply to Hans Leidekker from comment #13)
> Does the bug reappear if you revert this commit?

No, it doesn't

[Bug 51790] MSI is no longer able to detect correct architecture for a custom action DLL

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51790

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #15 from Alexandre Julliard  ---
Closing bugs fixed in 7.0-rc2.