Closed Bug 1523765 Opened 6 years ago Closed 6 years ago

InterceptionManager::PatchClientFunctions() assumes child/parent are both 32 or both 64 bit

Categories

(Core :: Security: Process Sandboxing, defect, P1)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: cpearce, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

I'm trying to get an aarch64 firefox parent process to startup an x86 plugin-container process to run an x86 GMP plugin so we can do Widevine/EME on aarch64 without waiting for Widevine to give us an aarch64 CDM.

Currently we're failing to init the sandbox during interception setup, because InterceptionManager::PatchClientFunctions() assumes that the parent and the child process are both 32 or both 64 bit, but in this case we have a 64bit (aarch64) parent, and a 32 bit x86 child....

Here's the call stack where we're failing:

firefox.exe!sandbox::InterceptionManager::PatchClientFunctions(sandbox::DllInterceptionData * thunks, unsigned __int64 thunk_bytes, sandbox::DllInterceptionData * dll_data) Line 498 C++
firefox.exe!sandbox::InterceptionManager::PatchNtdll(bool) Line 437 C++
firefox.exe!sandbox::InterceptionManager::InitializeInterceptions() Line 150  C++
firefox.exe!sandbox::PolicyBase::SetupAllInterceptions(sandbox::TargetProcess * target) Line 676  C++
firefox.exe!sandbox::PolicyBase::AddTarget(sandbox::TargetProcess * target) Line 543  C++
firefox.exe!sandbox::BrokerServicesBase::SpawnTarget(const wchar_t * exe_path, const wchar_t * command_line, std::map<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,std::less<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > >,std::allocator<std::pair<const std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > > > > & env_map, scoped_refptr<sandbox::TargetPolicy> policy, sandbox::ResultCode * last_warning, unsigned long * last_error, _PROCESS_INFORMATION * target_info) Line 476 C++
xul.dll!mozilla::SandboxBroker::LaunchApp(const wchar_t * aPath, const wchar_t * aArguments, std::map<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,std::less<std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > >,std::allocator<std::pair<const std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >,std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> > > > > & aEnvironment, GeckoProcessType aProcessType, const bool aProcessHandle, void * *) Line 235 C++
xul.dll!mozilla::ipc::GeckoChildProcessHost::PerformAsyncLaunch(std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > > aExtraOpts) Line 1045  C++
xul.dll!mozilla::ipc::GeckoChildProcessHost::RunPerformAsyncLaunch(std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > > aExtraOpts) Line 479  C++
xul.dll!mozilla::detail::RunnableMethodImpl<mozilla::ipc::GeckoChildProcessHost *,bool (mozilla::ipc::GeckoChildProcessHost::*)(std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > >),0,mozilla::RunnableKind::Standard,std::vector<std::basic_string<char,std::char_traits<char>,std::allocator<char> >,std::allocator<std::basic_string<char,std::char_traits<char>,std::allocator<char> > > > >::Run() Line 1161 C++
xul.dll!MessageLoop::RunTask(already_AddRefed<nsIRunnable> aTask) Line 443  C++
xul.dll!MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask && pending_task) Line 450 C++
xul.dll!MessageLoop::DoWork() Line 523  C++
xul.dll!base::MessagePumpForIO::DoRunLoop() Line 422  C++
xul.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 79 C++
xul.dll!MessageLoop::RunHandler() Line 309  C++
xul.dll!base::Thread::ThreadMain() Line 192 C++
xul.dll!`anonymous namespace'::ThreadFunc(void * closure) Line 31 C++
[External Code] 

I have tried compiling the 32 bit ServiceResolverThunks (service_resolver_32.cc) with the intention of using it from the 64bit code to install the interceptions in the 32bit process, but I'm getting a bunch of static compile time asserts related to the size of addresses not matching up, and that makes me doubt it will Just Work.

Here's the static assert failures:

$ sh aarch64-dgb-deploy.sh
 0:00.81 c:\mozilla-build\bin\mozmake.EXE -C c:/Users/chris/src/firefox/obj-aarch64-dbg -j28 -s backend
 0:01.14 Build configuration changed. Regenerating backend.
 0:05.15  0:03.71 File already read. Skipping: c:/Users/chris/src/firefox/gfx/angle/targets/angle_common/moz.build
 0:05.19  0:03.75 File already read. Skipping: c:/Users/chris/src/firefox/gfx/angle/targets/angle_common/moz.build
 0:05.19  0:03.76 File already read. Skipping: c:/Users/chris/src/firefox/gfx/angle/targets/angle_common/moz.build
 0:05.20  0:03.76 File already read. Skipping: c:/Users/chris/src/firefox/gfx/angle/targets/angle_common/moz.build
 0:05.20  0:03.76 File already read. Skipping: c:/Users/chris/src/firefox/gfx/angle/targets/translator/moz.build
 0:05.20  0:03.76 File already read. Skipping: c:/Users/chris/src/firefox/gfx/angle/targets/libGLESv2/moz.build
 0:18.83 Reticulating splines...
 0:18.83 Finished reading 1723 moz.build files in 3.53s
 0:18.83 Read 66 gyp files in parallel contributing 0.00s to total wall time
 0:18.83 Processed into 8995 build config descriptors in 3.96s
 0:18.83 RecursiveMake backend executed in 5.07s
 0:18.83   3105 total backend files; 0 created; 7 updated; 3098 unchanged; 0 deleted; 29 -> 1161 Makefile
 0:18.83 FasterMake backend executed in 0.35s
 0:18.83   11 total backend files; 0 created; 0 updated; 11 unchanged; 0 deleted
 0:18.83 VisualStudio backend executed in 1.61s
 0:18.83 Generated Visual Studio solution at c:/Users/chris/src/firefox/obj-aarch64-dbg\msvc\mozilla.sln
 0:18.83 Total wall time: 17.13s; CPU time: 17.13s; Efficiency: 100%; Untracked: 2.61s
 0:19.26 c:\mozilla-build\bin\mozmake.EXE -j28 -s binaries
 0:21.70 Elapsed: 1.62s; From dist/include: Kept 0 existing; Added/updated 5548; Removed 0 files and 0 directories.
 0:22.05 js/src/rust/force-cargo-library-build
 0:22.05 toolkit/library/rust/force-cargo-library-build
 0:22.05 js/src/frontend/binsource/force-cargo-host-program-build
 0:22.30 security/sandbox
 0:23.28     Blocking waiting for file lock on build directory
 0:23.34     Finished dev [optimized + debuginfo] target(s) in 1.04s
 0:23.66     Finished dev [optimized + debuginfo] target(s) in 1.36s
 0:23.84     Finished dev [optimized + debuginfo] target(s) in 1.55s
 0:28.82 In file included from c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc:11:
 0:28.82 c:/Users/chris/src/firefox/security/sandbox/chromium\base/bit_cast.h(65,3):  error: static_assert failed due to requirement 'sizeof(const voi
g)' "bit_cast requires source and destination to be the same size"
 0:28.82   static_assert(sizeof(Dest) == sizeof(Source),
 0:28.82   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:28.82 c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc(249,30):  note: in instantiation of function temp
st<const void *, unsigned long>' requested here
 0:28.82                              bit_cast<const void*>(function_code.stub),
 0:28.82                              ^
 0:28.82 In file included from c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc:11:
 0:28.82 c:/Users/chris/src/firefox/security/sandbox/chromium\base/bit_cast.h(65,3):  error: static_assert failed due to requirement 'sizeof(const wch
long)' "bit_cast requires source and destination to be the same size"
 0:28.82   static_assert(sizeof(Dest) == sizeof(Source),
 0:28.83   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:28.83 c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc(261,28):  note: in instantiation of function temp
st<const wchar_t *, unsigned long>' requested here
 0:28.83                            bit_cast<const wchar_t*>(ki_system_call),
 0:28.83                            ^
 0:28.87 In file included from c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc:11:
 0:28.88 c:/Users/chris/src/firefox/security/sandbox/chromium\base/bit_cast.h(65,3):  error: static_assert failed due to requirement 'sizeof(unsigned
t_cast requires source and destination to be the same size"
 0:28.90   static_assert(sizeof(Dest) == sizeof(Source),
 0:28.94   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:28.95 c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc(303,27):  note: in instantiation of function temp
st<unsigned long, int *>' requested here
 0:28.95   intercepted_code.stub = bit_cast<ULONG>(&full_remote_thunk->internal_thunk);
 0:28.95                           ^
 0:28.95 In file included from c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc:11:
 0:28.95 c:/Users/chris/src/firefox/security/sandbox/chromium\base/bit_cast.h(65,3):  error: static_assert failed due to requirement 'sizeof(unsigned
it_cast requires source and destination to be the same size"
 0:28.95   static_assert(sizeof(Dest) == sizeof(Source),
 0:28.99   ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:29.00 c:/Users/chris/src/firefox/security/sandbox/chromium/sandbox/win/src/service_resolver_32.cc(361,17):  note: in instantiation of function temp
st<unsigned long, void *>' requested here
 0:29.00     relative += bit_cast<ULONG>(target_) - bit_cast<ULONG>(remote_thunk);
 0:29.00                 ^
 0:29.01 4 errors generated.
 0:29.01 mozmake.EXE[2]: *** [c:/Users/chris/src/firefox/config/rules.mk:1107: service_resolver_32.obj] Error 1
 0:29.01 mozmake.EXE[2]: *** Waiting for unfinished jobs....
 0:30.45 c:/Users/chris/src/firefox/security/sandbox/win/SandboxInitialization.cpp(134,23):  warning: unused variable 'result' [-Wunused-variable]
 0:30.45   sandbox::ResultCode result = policy->CreateAlternateDesktop(true);
 0:30.45                       ^
 0:30.66 1 warning generated.
 0:30.68 mozmake.EXE[1]: *** [c:/Users/chris/src/firefox/config/recurse.mk:74: security/sandbox/target] Error 2
 0:30.70 mozmake.EXE[1]: *** Waiting for unfinished jobs....
 0:30.81 mozmake.EXE: *** [c:/Users/chris/src/firefox/config/recurse.mk:39: binaries] Error 2
 0:30.86 227 compiler warnings present.

Bob: Any ideas here? Do you think it would be possible to run the 32 bit ServiceResolverThunk's in the 64 bit parent process?

Flags: needinfo?(bobowencode)
Attached file aarch64-firefox-x86-gmp.bundle (obsolete) (deleted) —

hg bundle of my patch queue to make an aarch64 firefox try to instantiate an x86 gmp-clearkey.

To build with this, apply the bundle, and build an i686 build and in a separate objdir build an aarch64 build. ./mach package both builds, and copy the contents of $obj-i686/dist/firefox/ to $obj-aarch64/dist/firefox/i686/. So the path to gmp-clearkey should therefore be $obj-aarch64/dist/firefox/i686/gmp-clearkey/0.1/clearkey.dll. Then copy $obj-aarch64/dist/firefox to your aarch64 device, and run that, and open https://cpearce.github.io/mse-eme/.

Note: 64bit Windows builds are now the default; if you have an up to date m-c tree you'll need to add:

ac_add_options --target=i686

https://groups.google.com/forum/#!topic/mozilla.dev.platform/q8m7Hvdr87E

(In reply to Chris Pearce [:cpearce (GMT+13)] from comment #1)

Bob: Any ideas here? Do you think it would be possible to run the 32 bit ServiceResolverThunk's in the 64 bit parent process?

My guess is that it is probably possible, but I have no real idea over how much work it would be off the top of my head.
We could well find that it's not just the function patching, but the sandbox IPC code will need to be checked to make sure it doesn't use types that would be different sizes in the two processes as well.

Flags: needinfo?(bobowencode)
Priority: -- → P2
Flags: needinfo?(bobowencode)
Attached file widevine-aarch64.bundle (deleted) —

Bundle of changes needed to make Widevine and ClearKey work on aarch64.

Steps to run this are the same as in comment 2 above, difference is that Widevine works, and this bundle doesn't include my extra logging which caused issues with the content process sandbox.

Netflix works with these patches applied (and the GMP sandbox disabled).

Attachment #9039933 - Attachment is obsolete: true

I'm looking at trying to get this to work, but it requires quite a few changes to the chromium sandboxed child initialization code and the shared memory IPC client.

Current issues:

  • Compile time macros used to determine names of patched functions and their replacements - should be straight forward to make dynamic based on child type.
  • Resolving code to find these functions assumes same bitness - creating 32-bit versions of the code in the 64-bit build shouldn't be too difficult.
  • Resolving the replacement function addresses also relies on loading the child exe - a 32-bit exe can't normally be loaded in 64-bit process, but looks like it can be loaded as a data file and we should still be able to get the replacement function resolving to work. This resolving is done relative to the base address of the exe, which we already get from the child Process Environment Block.
  • Resolving the addresses for the patched functions in ntdll.dll which is different for 32-bit - again we can't load normally and the current code relies on ntdll.dll being loaded at the same address in the current process. This won't be the case when we load it as a data file. Current idea it to have a small helper exe to do a one-time look-up for the 32-bit ntdll.dll base address.
  • More resolving of addresses required for the transferring of variables - should be covered by or similar to above changes
  • Shared memory IPC client uses types that have different sizes between to 32-bit and 64-bit processes - not fully clear on the extent of this, my hope is that I can pad the structures on the 32-bit side to minimise the ripple effect of any changes.

So, there is a lot of fairly delicate work here and there are probably things I haven't found yet.
I hope to get something close to working by the end of next week, but that's possibly quite optimistic.

Assignee: nobody → bobowencode
Status: NEW → ASSIGNED
Flags: needinfo?(bobowencode)
Priority: P2 → P1

We're going with bug 1530245 as the solution here.

Assignee: bobowencode → nobody
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: