[Bug 45642] New: chrome/chromium sandbox needs proper x64 api thunks
https://bugs.winehq.org/show_bug.cgi?id=45642 Bug ID: 45642 Summary: chrome/chromium sandbox needs proper x64 api thunks Product: Wine Version: 3.13 Hardware: x86 OS: Linux Status: NEW Keywords: patch Severity: normal Priority: P2 Component: ntdll Assignee: wine-bugs(a)winehq.org Reporter: dark.shadow4(a)web.de Distribution: --- Created attachment 62072 --> https://bugs.winehq.org/attachment.cgi?id=62072 0001-ntdll-Provide-hookable-64bit-native-api-thunks For completeness sake, split of bug 21232. The current staging implementation doesn't supply the x64 syscall thunks the chromium sandbox expects. Patch attached. Could probably need some testing on mac systems, since I just ignored that. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |45643 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=21232 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com --- Comment #1 from Fabian Maurer <dark.shadow4(a)web.de> --- As noted, a bit of testing would make sense to mac, but I don't own one. Could we get this into staging then? Also, I planned on improving this patch to only use those thunks when actually needed, but that didn't turn out to work. So we have to apply them all the time, or chromium won't be happy. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|chrome/chromium sandbox |Chrome/Chromium sandbox |needs proper x64 api thunks |needs x86-64 syscall thunks | |to match Windows -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62072|0 |1 is obsolete| | --- Comment #2 from Fabian Maurer <dark.shadow4(a)web.de> --- Created attachment 62923 --> https://bugs.winehq.org/attachment.cgi?id=62923 Provide hookable 64bit api thunks Updated the patch to fix compile error on BSD/Mac. Also managed to get Mac support tested out, it allows x64 Chrome to run. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #3 from Fabian Maurer <dark.shadow4(a)web.de> --- Created attachment 62926 --> https://bugs.winehq.org/attachment.cgi?id=62926 Provide hookable 64bit api thunks (simpler method) Interesting enough, I found another approach which is far less invasive: Chromium doesn't check that there is an int 2e in win10 syscall thunks - so we can just add a jump in there. I figured it would fail since chromium copies the contents of the thunk, and our jump would fail - but that doesn't seem to be the case. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Louis Lenders <xerox.xerox2000x(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |xerox.xerox2000x(a)gmail.com --- Comment #4 from Louis Lenders <xerox.xerox2000x(a)gmail.com> --- Hi Fabian Do you have an update of the patch in comment3; it doesn`t apply anymore and I`d like to try it. Thanks in advance -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #62926|0 |1 is obsolete| | --- Comment #5 from Fabian Maurer <dark.shadow4(a)web.de> --- Created attachment 63951 --> https://bugs.winehq.org/attachment.cgi?id=63951 Provide hookable 64bit api thunks (simpler method) 2 No problem, added an updated patch. Should apply cleanly on top of latest wine-staging now. Still would love to get it into staging though, maybe I should edit a bit and ask again. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #6 from Louis Lenders <xerox.xerox2000x(a)gmail.com> --- (In reply to Fabian Maurer from comment #5)
Created attachment 63951 [details] Provide hookable 64bit api thunks (simpler method) 2
No problem, added an updated patch. Should apply cleanly on top of latest wine-staging now. Still would love to get it into staging though, maybe I should edit a bit and ask again.
Thanks! I`ll give it a try. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 pattietreutel <katyaberezyaka(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katyaberezyaka(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Louis Lenders <xerox.xerox2000x(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lenovolobo933(a)gmail.com --- Comment #7 from Louis Lenders <xerox.xerox2000x(a)gmail.com> --- *** Bug 46147 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Fabian Maurer <dark.shadow4(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63951|0 |1 is obsolete| | --- Comment #8 from Fabian Maurer <dark.shadow4(a)web.de> --- Created attachment 64059 --> https://bugs.winehq.org/attachment.cgi?id=64059 Patch to provide hookable api thunks (simpler method) 3 Simplified my patch to fix the issue. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED Fixed by SHA1| |5b5978875218b2851b8591c8b59 | |517a8502890b3 --- Comment #9 from Zebediah Figura <z.figura12(a)gmail.com> --- This has been committed as <https://github.com/wine-staging/wine-staging/commit/5b5978875218b2851b8591c8b59517a8502890b3>. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |UNCONFIRMED Ever confirmed|1 |0 Resolution|FIXED |--- --- Comment #10 from Zebediah Figura <z.figura12(a)gmail.com> --- Ech, actually, let's keep this open as a bug against Wine. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #11 from Fabian Maurer <dark.shadow4(a)web.de> --- @Zebediah Figura Something must have gone wrong with the patch, it completely breaks 64bit wine. You can't just use
user_shared_data_external->SystemCallPad[0] = 1 You need to have both: user_shared_data->SystemCallPad[0] = 1; user_shared_data_external->SystemCallPad[0] = 1;
-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #12 from Zebediah Figura <z.figura12(a)gmail.com> --- Fixed, but this raises a concern. This means that we will always start the shared data update thread on 64-bit. That's expensive and really should be avoided if possible. Do we still even know of an application, besides Chromium itself, that relies on this? If not I'm inclined to take this modification back out of Staging, and then just lobby Chromium to fix their hooking API. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #13 from Fabian Maurer <dark.shadow4(a)web.de> --- I don't follow, why does this mean we need to start a thread? All we need is the "1" at the right place in memory. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #14 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Fabian Maurer from comment #13)
I don't follow, why does this mean we need to start a thread? All we need is the "1" at the right place in memory.
"The right place in memory" is the KSHARED_USER_DATA page. This page notably includes timestamps which must be kept up to date; see bug 29168 and the attached Staging patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #15 from Fabian Maurer <dark.shadow4(a)web.de> --- Ah, so the user update thread is only started when that structure is accessed, did I get that right? Can't we just start the thread when the timestamps are accessed? Or is there some technical reason we need to start the thread even when accessing some other part of the structure? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #16 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Fabian Maurer from comment #15)
Ah, so the user update thread is only started when that structure is accessed, did I get that right?
Can't we just start the thread when the timestamps are accessed? Or is there some technical reason we need to start the thread even when accessing some other part of the structure?
The way we can tell when the thread is started is by marking the page no-access and then setting a SEGV handler. That would also be expensive. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #17 from Fabian Maurer <dark.shadow4(a)web.de> --- Created attachment 64559 --> https://bugs.winehq.org/attachment.cgi?id=64559 Workaround for user shared data issue Ah, I misunderstood. I though memory permissions could be set more granular, but if it always affects whole pages this would be too expensive... How about moving the SystemCallPad flag somewhere else? I attached a PoC, this also makes chrome happy. We just need to set some fixed address to the value 1 and refer to it in the syscall thunks. Would this be feasable? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #18 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Fabian Maurer from comment #17)
Created attachment 64559 [details] Workaround for user shared data issue
Ah, I misunderstood. I though memory permissions could be set more granular, but if it always affects whole pages this would be too expensive...
How about moving the SystemCallPad flag somewhere else? I attached a PoC, this also makes chrome happy. We just need to set some fixed address to the value 1 and refer to it in the syscall thunks. Would this be feasable?
That's feasible, yes. Still, is there any reason we can't just lobby Chromium to fix their software? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #19 from Fabian Maurer <dark.shadow4(a)web.de> ---
Still, is there any reason we can't just lobby Chromium to fix their software?
Honestly, I don't know how likely a fix is. Do they also cater to wine? And then there's the issue that it will take quite some time until the change has spread. I personally just want it to be as close to windows as possible, in case other programs are doing something similar. Or if someone wants to run old chromium based programs which don't have the fix yet. But then again, it's not my decision. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 --- Comment #20 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Fabian Maurer from comment #19)
Still, is there any reason we can't just lobby Chromium to fix their software?
Honestly, I don't know how likely a fix is. Do they also cater to wine?
We don't know until we ask.
And then there's the issue that it will take quite some time until the change has spread. I personally just want it to be as close to windows as possible, in case other programs are doing something similar. Or if someone wants to run old chromium based programs which don't have the fix yet. But then again, it's not my decision.
This is why I asked if there are any applications other than Chromium itself that actually rely on this working. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Alex Henrie <alexhenrie24(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=42388 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Kevin Locke <kevin(a)kevinlocke.name> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kevin(a)kevinlocke.name --- Comment #21 from Kevin Locke <kevin(a)kevinlocke.name> --- (In reply to Zebediah Figura from comment #20)
This is why I asked if there are any applications other than Chromium itself that actually rely on this working.
Bug 42388 comment 7 indicates that Firefox also relies on this working. Presumably other Blink-based (e.g. Opera, Brave, Vivaldi, Edge) and Gecko-based (e.g. TBB) browsers are affected. I'm currently trying to run Firefox or Chrome in Wine and curious about the state of this issue. It appears that the patch from attachment 64059 was removed from wine-staging in https://github.com/wine-staging/wine-staging/commit/5b5978875218b2851b8591c8... and neither it nor attachment 64559 applies to the current Wine master. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED Fixed by SHA1|5b5978875218b2851b8591c8b59 |917a206b01c82170a862e8497cb |517a8502890b3 |e26b6f1bfade0 --- Comment #22 from Zebediah Figura <z.figura12(a)gmail.com> --- This essentially should have been fixed by 917a206b01c82170a862e8497cbe26b6f1bfade0. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45642 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #23 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 5.17. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (2)
-
wine-bugs＠winehq.org -
WineHQ Bugzilla