Regarding x86 builds: I did a similar experiment with the isolated example from bug 1783223 comment 6. ``` bp KERNELBASE!VirtualProtect+0x39 "j (eax=0) ''; 'gc'" bp KERNELBASE!VirtualAlloc+0x4c "j (eax=0) ''; 'gc'" bp KERNELBASE!SetThreadInformation "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!GetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!SetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" ``` This led me to the code in `msmpeg2vdec` that looks like the following, the translation to C++ being my own (see attached translation with assembly included if curious): ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bGetResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto opt_out; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); someObject->someField = 1; opt_out_done: return someObject; ``` The important point here is that the result from `SetThreadInformation` is lost and not used to check for failure. The code seems to assume that opting out will work. ``` DWORD dwThreadDynamicCodePolicy = 0; 656f5985 8364240c00 and dword ptr [esp+0Ch],0 BOOL bGetResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f598a 8d44240c lea eax,[esp+0Ch] 656f598e 6a04 push 4 656f5990 50 push eax 656f5991 6a02 push 2 656f5993 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f5999 8b35e81a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x146688 (65851ae8)] // points to GetThreadInformation 656f599f 8bce mov ecx,esi 656f59a1 50 push eax 656f59a2 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret 656f59a8 ffd6 call esi if(!bResult) { goto opt_out; } 656f59aa 85c0 test eax,eax 656f59ac 740b je msmpeg2vdec!DllRegisterServer+0x24519 (656f59b9) if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } 656f59ae 837c240c01 cmp dword ptr [esp+0Ch],1 656f59b3 0f849845fcff je msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; 656f59b9 0fb6c3 movzx eax,bl 656f59bc 8944240c mov dword ptr [esp+0Ch],eax SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f59c0 8d44240c lea eax,[esp+0Ch] 656f59c4 6a04 push 4 656f59c6 50 push eax 656f59c7 6a02 push 2 656f59c9 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] 656f59cf 8b35ec1a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x14668c (65851aec)] 656f59d5 8bce mov ecx,esi 656f59d7 50 push eax 656f59d8 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] 656f59de ffd6 call esi someObject->someField = 1; 656f59e0 c6471001 mov byte ptr [edi+10h],1 656f59e4 e96845fcff jmp msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out_done: return someObject; 656b9f51 8bc7 mov eax,edi 656b9f53 5f pop edi 656b9f54 5e pop esi 656b9f55 5b pop ebx 656b9f56 8be5 mov esp,ebp 656b9f58 5d pop ebp 656b9f59 c20400 ret 4 ``` ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto error; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto good; } dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); good: ...; ```
Bug 1783223 Comment 23 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
It seems I submitted my ongoing next comment before it was complete, sorry about that.
Regarding x86 builds: I did a similar experiment with the isolated example from bug 1783223 comment 6 (not Firefox). I would agree that given the current 32-bit version of `msmpeg2vdec`, the changes that [:jrmuizel] introduced to allow threads to opt out are the best compromise we can make for 32-bit builds. Here are more details, obtained with the following breakpoints: ``` bp KERNELBASE!VirtualProtect+0x39 "j (eax=0) ''; 'gc'" bp KERNELBASE!VirtualAlloc+0x4c "j (eax=0) ''; 'gc'" bp KERNELBASE!SetThreadInformation "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!GetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!SetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" ``` This led me to a code path in `msmpeg2vdec` which looks as follows, the translation to C++ being my own: ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto opt_out; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); someObject->someField = 1; opt_out_done: return someObject; ``` Here is the same with the original assembly: ``` DWORD dwThreadDynamicCodePolicy = 0; 656f5985 8364240c00 and dword ptr [esp+0Ch],0 BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f598a 8d44240c lea eax,[esp+0Ch] 656f598e 6a04 push 4 656f5990 50 push eax 656f5991 6a02 push 2 656f5993 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f5999 8b35e81a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x146688 (65851ae8)] // points to GetThreadInformation 656f599f 8bce mov ecx,esi 656f59a1 50 push eax 656f59a2 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59a8 ffd6 call esi if(!bResult) { goto opt_out; } 656f59aa 85c0 test eax,eax 656f59ac 740b je msmpeg2vdec!DllRegisterServer+0x24519 (656f59b9) if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } 656f59ae 837c240c01 cmp dword ptr [esp+0Ch],1 656f59b3 0f849845fcff je msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; 656f59b9 0fb6c3 movzx eax,bl 656f59bc 8944240c mov dword ptr [esp+0Ch],eax SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f59c0 8d44240c lea eax,[esp+0Ch] 656f59c4 6a04 push 4 656f59c6 50 push eax 656f59c7 6a02 push 2 656f59c9 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f59cf 8b35ec1a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x14668c (65851aec)] // points to SetThreadInformation 656f59d5 8bce mov ecx,esi 656f59d7 50 push eax 656f59d8 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59de ffd6 call esi someObject->someField = 1; 656f59e0 c6471001 mov byte ptr [edi+10h],1 656f59e4 e96845fcff jmp msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out_done: return someObject; 656b9f51 8bc7 mov eax,edi 656b9f53 5f pop edi 656b9f54 5e pop esi 656b9f55 5b pop ebx 656b9f56 8be5 mov esp,ebp 656b9f58 5d pop ebp 656b9f59 c20400 ret 4 ``` We reach this code after multiple code paths that get the current process' policy for ACG, including one originating from `msmpeg2vdec` (note that this was not the case with 64-bit builds). The important point I'd like to share about the code above is that the result of the call to `SetThreadInformation` is lost. This code thums seems to assume that opting out of ACG will work and seems unable to adapt to a strict ACG without opt-out. After this code gets executed, a failing call to `VirtualProtect` occurs, originating from `msmpeg2vdec` occurs: ``` 00 03afd5f0 656b938e KERNELBASE!VirtualProtect+0x39 01 03afd618 656b932e msmpeg2vdec!DllGetClassObject+0x8aee 02 03afd630 656b4b90 msmpeg2vdec!DllGetClassObject+0x8a8e 03 03afd7e8 656aec59 msmpeg2vdec!DllGetClassObject+0x42f0 04 03afd8d8 656ae894 msmpeg2vdec+0x7ec59 05 03afd918 656ae3fa msmpeg2vdec+0x7e894 06 03afd950 656ae373 msmpeg2vdec+0x7e3fa 07 03afd968 75a09d5c msmpeg2vdec+0x7e373 08 03afda68 75a17385 combase!CServerContextActivator::CreateInstance+0x1ec [onecore\com\combase\objact\actvator.cxx @ 881] 09 03afdab4 75a0959f combase!ActivationPropertiesIn::DelegateCreateInstance+0xf5 [onecore\com\combase\actprops\actprops.cxx @ 1931] 0a 03afdb14 75a08d9a combase!CApartmentActivator::CreateInstance+0xbf [onecore\com\combase\objact\actvator.cxx @ 2189] 0b 03afdb40 75a841e5 combase!CProcessActivator::CCICallback+0x5a [onecore\com\combase\objact\actvator.cxx @ 1645] 0c 03afdb60 75a8409d combase!CProcessActivator::AttemptActivation+0x35 [onecore\com\combase\objact\actvator.cxx @ 1527] 0d 03afdba0 75a84021 combase!CProcessActivator::ActivateByContext+0x6d [onecore\com\combase\objact\actvator.cxx @ 1393] 0e 03afdbd0 75a1734d combase!CProcessActivator::CreateInstance+0x61 [onecore\com\combase\objact\actvator.cxx @ 1271] 0f 03afdc1c 75a17b8d combase!ActivationPropertiesIn::DelegateCreateInstance+0xbd [onecore\com\combase\actprops\actprops.cxx @ 1931] 10 03afde80 75a17354 combase!CClientContextActivator::CreateInstance+0xfd [onecore\com\combase\objact\actvator.cxx @ 570] 11 03afdecc 75a3f632 combase!ActivationPropertiesIn::DelegateCreateInstance+0xc4 [onecore\com\combase\actprops\actprops.cxx @ 1983] 12 03afea10 75a3e8a1 combase!ICoCreateInstanceEx+0xc12 [onecore\com\combase\objact\objact.cxx @ 2032] 13 03afeb04 75a3e60e combase!CComActivator::DoCreateInstance+0x231 [onecore\com\combase\objact\immact.hxx @ 401] 14 (Inline) -------- combase!CoCreateInstanceEx+0xa2 [onecore\com\combase\objact\actapi.cxx @ 177] 15 03afeb44 653cb83f combase!CoCreateInstance+0xbe [onecore\com\combase\objact\actapi.cxx @ 121] 16 03afec14 653cb718 MFPlat!CMFTransformActivate::InstantiateTransform+0xdf 17 03afec30 653cb665 MFPlat!CMFTransformActivate::InstantiateMediaObject+0x38 18 03afec48 653cb5e4 MFPlat!CMFActivate::DoActivate+0x45 19 03afec64 78e514ae MFPlat!CMFActivate::ActivateObject+0x34 1a 03afedc4 78e50e72 mfcore!CTopoCodecEnumEx::GetTransform+0x58e 1b 03afee54 78e51eb3 mfcore!CTopoCodecEnumEx::FindCodec+0xc5 1c 03aff000 78e3929d mfcore!CTopoConnector::FindDecoder+0x623 1d 03aff13c 78e382b5 mfcore!CTopoConnector::AddDecoderForMediaType+0x6cd 1e 03aff20c 78e37ef6 mfcore!CTopoConnector::AddDecoder+0x2b6 1f 03aff2ac 78e37927 mfcore!CTopoConnector::ConnectNodesUsingDecoder+0x382 20 03aff304 78e4c92d mfcore!CTopoConnector::ConnectNodesInternal+0x25e 21 03aff420 78e371f7 mfcore!CTopoConnector::ConnectNodes+0x30d 22 03aff4fc 78e4de32 mfcore!CMFTopoLoader::ExecuteConnectTask+0x6d7 23 03aff540 78e5bec0 mfcore!CMFTopoLoader::ExecuteConnectTasks+0x10b 24 03aff594 78e5b5dd mfcore!CMFTopoLoader::InternalLoad+0x25a 25 03aff5f8 78e86e6e mfcore!CMFTopoLoader::Load+0x44d 26 03aff6a4 78e78589 mfcore!CMediaSession::OpQueueTopology+0x5f6 27 03aff6dc 78e6bee7 mfcore!CMediaSession::DispatchOperation+0xf9 28 03aff6fc 78e6bda1 mfcore!COpQueue::ProcessMarshalledOperations+0x140 29 03aff708 6537573e mfcore!COpQueue::ProcessMarshalledOperationsAsyncCallback::Invoke+0x11 2a 03aff738 6537533d RTWorkQ!CSerialWorkQueue::QueueItem::ExecuteWorkItem+0x9e 2b 03aff75c 65373eef RTWorkQ!CSerialWorkQueue::QueueItem::OnWorkItemAsyncCallback::Invoke+0x1d 2c 03aff78c 774d6d94 RTWorkQ!ThreadPoolWorkCallback+0xcf 2d 03aff7c4 774d5e32 ntdll!TppWorkpExecuteCallback+0x144 2e 03aff978 772cfa29 ntdll!TppWorkerThread+0x472 2f 03aff988 77507b5e KERNEL32!BaseThreadInitThunk+0x19 30 03aff9e4 77507b2e ntdll!__RtlUserThreadStart+0x2f 31 03aff9f4 00000000 ntdll!_RtlUserThreadStart+0x1b ``` Then the code path that tries to opt out is reached once again (and will, again, fail without noticing at `SetThreadInformation`), and finally we crash by jumping to a portion of the area that `VirtualProtect` was trying to set as executable. In my case the failing call was `VirtualProtect(lpAddress=0x09160000, dwSize=0x00010000, flProtect=0x40=PAGE_EXECUTE_READWRITE)` and I was crashing with `eip=0916da87`.
Regarding 32-bit builds: I did a similar experiment with the isolated example from bug 1783223 comment 6 (not Firefox). I would agree that given the current 32-bit version of `msmpeg2vdec`, the changes that [:jrmuizel] introduced to allow threads to opt out are the best compromise we can make for 32-bit builds. Here are more details, obtained with the following breakpoints: ``` bp KERNELBASE!VirtualProtect+0x39 "j (eax=0) ''; 'gc'" bp KERNELBASE!VirtualAlloc+0x4c "j (eax=0) ''; 'gc'" bp KERNELBASE!SetThreadInformation "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!GetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!SetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" ``` This led me to a code path in `msmpeg2vdec` which looks as follows, the translation to C++ being my own: ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto opt_out; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); someObject->someField = 1; opt_out_done: return someObject; ``` Here is the same with the original assembly: ``` DWORD dwThreadDynamicCodePolicy = 0; 656f5985 8364240c00 and dword ptr [esp+0Ch],0 BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f598a 8d44240c lea eax,[esp+0Ch] 656f598e 6a04 push 4 656f5990 50 push eax 656f5991 6a02 push 2 656f5993 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f5999 8b35e81a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x146688 (65851ae8)] // points to GetThreadInformation 656f599f 8bce mov ecx,esi 656f59a1 50 push eax 656f59a2 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59a8 ffd6 call esi if(!bResult) { goto opt_out; } 656f59aa 85c0 test eax,eax 656f59ac 740b je msmpeg2vdec!DllRegisterServer+0x24519 (656f59b9) if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } 656f59ae 837c240c01 cmp dword ptr [esp+0Ch],1 656f59b3 0f849845fcff je msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; 656f59b9 0fb6c3 movzx eax,bl 656f59bc 8944240c mov dword ptr [esp+0Ch],eax SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f59c0 8d44240c lea eax,[esp+0Ch] 656f59c4 6a04 push 4 656f59c6 50 push eax 656f59c7 6a02 push 2 656f59c9 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f59cf 8b35ec1a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x14668c (65851aec)] // points to SetThreadInformation 656f59d5 8bce mov ecx,esi 656f59d7 50 push eax 656f59d8 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59de ffd6 call esi someObject->someField = 1; 656f59e0 c6471001 mov byte ptr [edi+10h],1 656f59e4 e96845fcff jmp msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out_done: return someObject; 656b9f51 8bc7 mov eax,edi 656b9f53 5f pop edi 656b9f54 5e pop esi 656b9f55 5b pop ebx 656b9f56 8be5 mov esp,ebp 656b9f58 5d pop ebp 656b9f59 c20400 ret 4 ``` We reach this code after multiple code paths that get the current process' policy for ACG, including one originating from `msmpeg2vdec` (note that this was not the case with 64-bit builds, where none was originating from `msmpeg2vdec`). The important point I'd like to share about the code above is that the result of the call to `SetThreadInformation` is not checked and thus lost. This code thums seems to assume that opting out of ACG will work and seems unable to adapt to a strict ACG without opt-out. After this code gets executed, a failing call to `VirtualProtect` occurs, originating from `msmpeg2vdec` occurs: ``` 00 03afd5f0 656b938e KERNELBASE!VirtualProtect+0x39 01 03afd618 656b932e msmpeg2vdec!DllGetClassObject+0x8aee 02 03afd630 656b4b90 msmpeg2vdec!DllGetClassObject+0x8a8e 03 03afd7e8 656aec59 msmpeg2vdec!DllGetClassObject+0x42f0 04 03afd8d8 656ae894 msmpeg2vdec+0x7ec59 05 03afd918 656ae3fa msmpeg2vdec+0x7e894 06 03afd950 656ae373 msmpeg2vdec+0x7e3fa 07 03afd968 75a09d5c msmpeg2vdec+0x7e373 08 03afda68 75a17385 combase!CServerContextActivator::CreateInstance+0x1ec [onecore\com\combase\objact\actvator.cxx @ 881] 09 03afdab4 75a0959f combase!ActivationPropertiesIn::DelegateCreateInstance+0xf5 [onecore\com\combase\actprops\actprops.cxx @ 1931] 0a 03afdb14 75a08d9a combase!CApartmentActivator::CreateInstance+0xbf [onecore\com\combase\objact\actvator.cxx @ 2189] 0b 03afdb40 75a841e5 combase!CProcessActivator::CCICallback+0x5a [onecore\com\combase\objact\actvator.cxx @ 1645] 0c 03afdb60 75a8409d combase!CProcessActivator::AttemptActivation+0x35 [onecore\com\combase\objact\actvator.cxx @ 1527] 0d 03afdba0 75a84021 combase!CProcessActivator::ActivateByContext+0x6d [onecore\com\combase\objact\actvator.cxx @ 1393] 0e 03afdbd0 75a1734d combase!CProcessActivator::CreateInstance+0x61 [onecore\com\combase\objact\actvator.cxx @ 1271] 0f 03afdc1c 75a17b8d combase!ActivationPropertiesIn::DelegateCreateInstance+0xbd [onecore\com\combase\actprops\actprops.cxx @ 1931] 10 03afde80 75a17354 combase!CClientContextActivator::CreateInstance+0xfd [onecore\com\combase\objact\actvator.cxx @ 570] 11 03afdecc 75a3f632 combase!ActivationPropertiesIn::DelegateCreateInstance+0xc4 [onecore\com\combase\actprops\actprops.cxx @ 1983] 12 03afea10 75a3e8a1 combase!ICoCreateInstanceEx+0xc12 [onecore\com\combase\objact\objact.cxx @ 2032] 13 03afeb04 75a3e60e combase!CComActivator::DoCreateInstance+0x231 [onecore\com\combase\objact\immact.hxx @ 401] 14 (Inline) -------- combase!CoCreateInstanceEx+0xa2 [onecore\com\combase\objact\actapi.cxx @ 177] 15 03afeb44 653cb83f combase!CoCreateInstance+0xbe [onecore\com\combase\objact\actapi.cxx @ 121] 16 03afec14 653cb718 MFPlat!CMFTransformActivate::InstantiateTransform+0xdf 17 03afec30 653cb665 MFPlat!CMFTransformActivate::InstantiateMediaObject+0x38 18 03afec48 653cb5e4 MFPlat!CMFActivate::DoActivate+0x45 19 03afec64 78e514ae MFPlat!CMFActivate::ActivateObject+0x34 1a 03afedc4 78e50e72 mfcore!CTopoCodecEnumEx::GetTransform+0x58e 1b 03afee54 78e51eb3 mfcore!CTopoCodecEnumEx::FindCodec+0xc5 1c 03aff000 78e3929d mfcore!CTopoConnector::FindDecoder+0x623 1d 03aff13c 78e382b5 mfcore!CTopoConnector::AddDecoderForMediaType+0x6cd 1e 03aff20c 78e37ef6 mfcore!CTopoConnector::AddDecoder+0x2b6 1f 03aff2ac 78e37927 mfcore!CTopoConnector::ConnectNodesUsingDecoder+0x382 20 03aff304 78e4c92d mfcore!CTopoConnector::ConnectNodesInternal+0x25e 21 03aff420 78e371f7 mfcore!CTopoConnector::ConnectNodes+0x30d 22 03aff4fc 78e4de32 mfcore!CMFTopoLoader::ExecuteConnectTask+0x6d7 23 03aff540 78e5bec0 mfcore!CMFTopoLoader::ExecuteConnectTasks+0x10b 24 03aff594 78e5b5dd mfcore!CMFTopoLoader::InternalLoad+0x25a 25 03aff5f8 78e86e6e mfcore!CMFTopoLoader::Load+0x44d 26 03aff6a4 78e78589 mfcore!CMediaSession::OpQueueTopology+0x5f6 27 03aff6dc 78e6bee7 mfcore!CMediaSession::DispatchOperation+0xf9 28 03aff6fc 78e6bda1 mfcore!COpQueue::ProcessMarshalledOperations+0x140 29 03aff708 6537573e mfcore!COpQueue::ProcessMarshalledOperationsAsyncCallback::Invoke+0x11 2a 03aff738 6537533d RTWorkQ!CSerialWorkQueue::QueueItem::ExecuteWorkItem+0x9e 2b 03aff75c 65373eef RTWorkQ!CSerialWorkQueue::QueueItem::OnWorkItemAsyncCallback::Invoke+0x1d 2c 03aff78c 774d6d94 RTWorkQ!ThreadPoolWorkCallback+0xcf 2d 03aff7c4 774d5e32 ntdll!TppWorkpExecuteCallback+0x144 2e 03aff978 772cfa29 ntdll!TppWorkerThread+0x472 2f 03aff988 77507b5e KERNEL32!BaseThreadInitThunk+0x19 30 03aff9e4 77507b2e ntdll!__RtlUserThreadStart+0x2f 31 03aff9f4 00000000 ntdll!_RtlUserThreadStart+0x1b ``` Then the code path that tries to opt out is reached once again (and will, again, fail without noticing at `SetThreadInformation`), and finally we crash by jumping to a portion of the area that `VirtualProtect` was trying to set as executable. In my case the failing call was `VirtualProtect(lpAddress=0x09160000, dwSize=0x00010000, flProtect=0x40=PAGE_EXECUTE_READWRITE)` and I was crashing with `eip=0916da87`.
Regarding 32-bit builds: I did a similar experiment with the isolated example from bug 1783223 comment 6 (not Firefox). I would agree that given the current behavior of the 32-bit version of `msmpeg2vdec.dll`, the changes that [:jrmuizel] introduced to allow threads to opt out are the best compromise we can make for 32-bit builds. Here are more details, obtained with the following breakpoints: ``` bp KERNELBASE!VirtualProtect+0x39 "j (eax=0) ''; 'gc'" bp KERNELBASE!VirtualAlloc+0x4c "j (eax=0) ''; 'gc'" bp KERNELBASE!SetThreadInformation "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!GetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!SetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" ``` This led me to a code path in `msmpeg2vdec` which looks as follows, the translation to C++ being my own: ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto opt_out; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); someObject->someField = 1; opt_out_done: return someObject; ``` Here is the same with the original assembly: ``` DWORD dwThreadDynamicCodePolicy = 0; 656f5985 8364240c00 and dword ptr [esp+0Ch],0 BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f598a 8d44240c lea eax,[esp+0Ch] 656f598e 6a04 push 4 656f5990 50 push eax 656f5991 6a02 push 2 656f5993 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f5999 8b35e81a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x146688 (65851ae8)] // points to GetThreadInformation 656f599f 8bce mov ecx,esi 656f59a1 50 push eax 656f59a2 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59a8 ffd6 call esi if(!bResult) { goto opt_out; } 656f59aa 85c0 test eax,eax 656f59ac 740b je msmpeg2vdec!DllRegisterServer+0x24519 (656f59b9) if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } 656f59ae 837c240c01 cmp dword ptr [esp+0Ch],1 656f59b3 0f849845fcff je msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; 656f59b9 0fb6c3 movzx eax,bl 656f59bc 8944240c mov dword ptr [esp+0Ch],eax SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f59c0 8d44240c lea eax,[esp+0Ch] 656f59c4 6a04 push 4 656f59c6 50 push eax 656f59c7 6a02 push 2 656f59c9 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f59cf 8b35ec1a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x14668c (65851aec)] // points to SetThreadInformation 656f59d5 8bce mov ecx,esi 656f59d7 50 push eax 656f59d8 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59de ffd6 call esi someObject->someField = 1; 656f59e0 c6471001 mov byte ptr [edi+10h],1 656f59e4 e96845fcff jmp msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out_done: return someObject; 656b9f51 8bc7 mov eax,edi 656b9f53 5f pop edi 656b9f54 5e pop esi 656b9f55 5b pop ebx 656b9f56 8be5 mov esp,ebp 656b9f58 5d pop ebp 656b9f59 c20400 ret 4 ``` We reach this code after multiple code paths that get the current process' policy for ACG, including one originating from `msmpeg2vdec` (note that this was not the case with 64-bit builds, where none was originating from `msmpeg2vdec`). The important point I'd like to share about the code above is that the result of the call to `SetThreadInformation` is not checked and thus lost. This code thums seems to assume that opting out of ACG will work and seems unable to adapt to a strict ACG without opt-out. After this code gets executed, a failing call to `VirtualProtect` occurs, originating from `msmpeg2vdec` occurs: ``` 00 03afd5f0 656b938e KERNELBASE!VirtualProtect+0x39 01 03afd618 656b932e msmpeg2vdec!DllGetClassObject+0x8aee 02 03afd630 656b4b90 msmpeg2vdec!DllGetClassObject+0x8a8e 03 03afd7e8 656aec59 msmpeg2vdec!DllGetClassObject+0x42f0 04 03afd8d8 656ae894 msmpeg2vdec+0x7ec59 05 03afd918 656ae3fa msmpeg2vdec+0x7e894 06 03afd950 656ae373 msmpeg2vdec+0x7e3fa 07 03afd968 75a09d5c msmpeg2vdec+0x7e373 08 03afda68 75a17385 combase!CServerContextActivator::CreateInstance+0x1ec [onecore\com\combase\objact\actvator.cxx @ 881] ... ``` Then the code path that tries to opt out is reached once again (and will, again, fail without noticing at `SetThreadInformation`), and finally we crash by jumping to a portion of the area that `VirtualProtect` was trying to set as executable. In my case the failing call was `VirtualProtect(lpAddress=0x09160000, dwSize=0x00010000, flProtect=0x40=PAGE_EXECUTE_READWRITE)` and I was crashing with `eip=0916da87`.
Regarding 32-bit builds: I did a similar experiment with the isolated example from bug 1783223 comment 6 (not Firefox). To summarize, I would agree that given the current behavior of the 32-bit version of `msmpeg2vdec.dll`, the changes that [:jrmuizel] introduced to allow threads to opt out are the best compromise we can make for 32-bit builds. Here are more details, obtained with the following breakpoints: ``` bp KERNELBASE!VirtualProtect+0x39 "j (eax=0) ''; 'gc'" bp KERNELBASE!VirtualAlloc+0x4c "j (eax=0) ''; 'gc'" bp KERNELBASE!SetThreadInformation "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!GetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!SetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" ``` This led me to a code path in `msmpeg2vdec` which looks as follows, the translation to C++ being my own: ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto opt_out; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); someObject->someField = 1; opt_out_done: return someObject; ``` Here is the same with the original assembly: ``` DWORD dwThreadDynamicCodePolicy = 0; 656f5985 8364240c00 and dword ptr [esp+0Ch],0 BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f598a 8d44240c lea eax,[esp+0Ch] 656f598e 6a04 push 4 656f5990 50 push eax 656f5991 6a02 push 2 656f5993 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f5999 8b35e81a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x146688 (65851ae8)] // points to GetThreadInformation 656f599f 8bce mov ecx,esi 656f59a1 50 push eax 656f59a2 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59a8 ffd6 call esi if(!bResult) { goto opt_out; } 656f59aa 85c0 test eax,eax 656f59ac 740b je msmpeg2vdec!DllRegisterServer+0x24519 (656f59b9) if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } 656f59ae 837c240c01 cmp dword ptr [esp+0Ch],1 656f59b3 0f849845fcff je msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; 656f59b9 0fb6c3 movzx eax,bl 656f59bc 8944240c mov dword ptr [esp+0Ch],eax SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f59c0 8d44240c lea eax,[esp+0Ch] 656f59c4 6a04 push 4 656f59c6 50 push eax 656f59c7 6a02 push 2 656f59c9 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f59cf 8b35ec1a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x14668c (65851aec)] // points to SetThreadInformation 656f59d5 8bce mov ecx,esi 656f59d7 50 push eax 656f59d8 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59de ffd6 call esi someObject->someField = 1; 656f59e0 c6471001 mov byte ptr [edi+10h],1 656f59e4 e96845fcff jmp msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out_done: return someObject; 656b9f51 8bc7 mov eax,edi 656b9f53 5f pop edi 656b9f54 5e pop esi 656b9f55 5b pop ebx 656b9f56 8be5 mov esp,ebp 656b9f58 5d pop ebp 656b9f59 c20400 ret 4 ``` We reach this code after multiple code paths that get the current process' policy for ACG, including one originating from `msmpeg2vdec` (note that this was not the case with 64-bit builds, where none was originating from `msmpeg2vdec`). The important point I'd like to share about the code above is that the result of the call to `SetThreadInformation` is not checked and thus lost. This code thums seems to assume that opting out of ACG will work and seems unable to adapt to a strict ACG without opt-out. After this code gets executed, a failing call to `VirtualProtect` occurs, originating from `msmpeg2vdec` occurs: ``` 00 03afd5f0 656b938e KERNELBASE!VirtualProtect+0x39 01 03afd618 656b932e msmpeg2vdec!DllGetClassObject+0x8aee 02 03afd630 656b4b90 msmpeg2vdec!DllGetClassObject+0x8a8e 03 03afd7e8 656aec59 msmpeg2vdec!DllGetClassObject+0x42f0 04 03afd8d8 656ae894 msmpeg2vdec+0x7ec59 05 03afd918 656ae3fa msmpeg2vdec+0x7e894 06 03afd950 656ae373 msmpeg2vdec+0x7e3fa 07 03afd968 75a09d5c msmpeg2vdec+0x7e373 08 03afda68 75a17385 combase!CServerContextActivator::CreateInstance+0x1ec [onecore\com\combase\objact\actvator.cxx @ 881] ... ``` Then the code path that tries to opt out is reached once again (and will, again, fail without noticing at `SetThreadInformation`), and finally we crash by jumping to a portion of the area that `VirtualProtect` was trying to set as executable. In my case the failing call was `VirtualProtect(lpAddress=0x09160000, dwSize=0x00010000, flProtect=0x40=PAGE_EXECUTE_READWRITE)` and I was crashing with `eip=0916da87`.
Regarding 32-bit builds: I did a similar experiment with the isolated example from bug 1783223 comment 6 (not Firefox). To summarize, I would agree that given the current behavior of the 32-bit version of `msmpeg2vdec.dll`, the changes that [:jrmuizel] introduced to allow threads to opt out are the best compromise we can make for 32-bit builds. Here are more details, obtained with the following breakpoints: ``` bp KERNELBASE!VirtualProtect+0x39 "j (eax=0) ''; 'gc'" bp KERNELBASE!VirtualAlloc+0x4c "j (eax=0) ''; 'gc'" bp KERNELBASE!SetThreadInformation "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!GetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" bp KERNELBASE!SetProcessMitigationPolicy "j (poi(esp+0x8) = 2) ''; 'gc'" ``` This led me to a code path in `msmpeg2vdec` which looks as follows, the translation to C++ being my own: ``` DWORD dwThreadDynamicCodePolicy = 0; BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); if(!bResult) { goto opt_out; } if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); someObject->someField = 1; opt_out_done: return someObject; ``` Here is the same with the original assembly: ``` DWORD dwThreadDynamicCodePolicy = 0; 656f5985 8364240c00 and dword ptr [esp+0Ch],0 BOOL bResult = GetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f598a 8d44240c lea eax,[esp+0Ch] 656f598e 6a04 push 4 656f5990 50 push eax 656f5991 6a02 push 2 656f5993 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f5999 8b35e81a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x146688 (65851ae8)] // points to GetThreadInformation 656f599f 8bce mov ecx,esi 656f59a1 50 push eax 656f59a2 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59a8 ffd6 call esi if(!bResult) { goto opt_out; } 656f59aa 85c0 test eax,eax 656f59ac 740b je msmpeg2vdec!DllRegisterServer+0x24519 (656f59b9) if(dwThreadDynamicCodePolicy == THREAD_DYNAMIC_CODE_ALLOW) { goto opt_out_done; } 656f59ae 837c240c01 cmp dword ptr [esp+0Ch],1 656f59b3 0f849845fcff je msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out: dwThreadDynamicCodePolicy = THREAD_DYNAMIC_CODE_ALLOW; 656f59b9 0fb6c3 movzx eax,bl 656f59bc 8944240c mov dword ptr [esp+0Ch],eax SetThreadInformation(GetCurrentThread(), ThreadDynamicCodePolicy, &dwThreadDynamicCodePolicy, sizeof(dwThreadDynamicCodePolicy)); 656f59c0 8d44240c lea eax,[esp+0Ch] 656f59c4 6a04 push 4 656f59c6 50 push eax 656f59c7 6a02 push 2 656f59c9 ff1594918565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14dd34 (65859194)] // points to GetCurrentThread 656f59cf 8b35ec1a8565 mov esi,dword ptr [msmpeg2vdec!DllUnregisterServer+0x14668c (65851aec)] // points to SetThreadInformation 656f59d5 8bce mov ecx,esi 656f59d7 50 push eax 656f59d8 ff15a8948565 call dword ptr [msmpeg2vdec!DllUnregisterServer+0x14e048 (658594a8)] // points to a ret instruction 656f59de ffd6 call esi someObject->someField = 1; 656f59e0 c6471001 mov byte ptr [edi+10h],1 656f59e4 e96845fcff jmp msmpeg2vdec!DllGetClassObject+0x96b1 (656b9f51) opt_out_done: return someObject; 656b9f51 8bc7 mov eax,edi 656b9f53 5f pop edi 656b9f54 5e pop esi 656b9f55 5b pop ebx 656b9f56 8be5 mov esp,ebp 656b9f58 5d pop ebp 656b9f59 c20400 ret 4 ``` We reach this code after multiple code paths that get the current process' policy for ACG, including one originating from `msmpeg2vdec` (note that this was not the case with 64-bit builds, where none was originating from `msmpeg2vdec`). The important point I'd like to share about the code above is that the result of the call to `SetThreadInformation` is not checked and thus lost. This code thums seems to assume that opting out of ACG will work and seems unable to adapt to a strict ACG without opt-out. After this code gets executed, a failing call to `VirtualProtect` originating from `msmpeg2vdec` occurs: ``` 00 03afd5f0 656b938e KERNELBASE!VirtualProtect+0x39 01 03afd618 656b932e msmpeg2vdec!DllGetClassObject+0x8aee 02 03afd630 656b4b90 msmpeg2vdec!DllGetClassObject+0x8a8e 03 03afd7e8 656aec59 msmpeg2vdec!DllGetClassObject+0x42f0 04 03afd8d8 656ae894 msmpeg2vdec+0x7ec59 05 03afd918 656ae3fa msmpeg2vdec+0x7e894 06 03afd950 656ae373 msmpeg2vdec+0x7e3fa 07 03afd968 75a09d5c msmpeg2vdec+0x7e373 08 03afda68 75a17385 combase!CServerContextActivator::CreateInstance+0x1ec [onecore\com\combase\objact\actvator.cxx @ 881] ... ``` Then the code path that tries to opt out is reached once again (and will, again, fail without noticing at `SetThreadInformation`), and finally we crash by jumping to a portion of the area that `VirtualProtect` was trying to set as executable. In my case the failing call was `VirtualProtect(lpAddress=0x09160000, dwSize=0x00010000, flProtect=0x40=PAGE_EXECUTE_READWRITE)` and I was crashing with `eip=0916da87`.