Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were loading the library; - but our (outdated) chromium sandbox code identifies this mapping as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so it will look for a string in the wrong location; - somehow ASAN realizes that the string in a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Bug 1818762 Comment 12 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so it will look for a string in the wrong location; - somehow ASAN realizes that the string in a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so it will look for a string in the wrong location; - somehow ASAN realizes that the string in a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we **do not** manage to find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so it will look for a string in the wrong location; - somehow ASAN realizes that the string in a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** manage to find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so it will look for a string in the wrong location; - somehow ASAN realizes that the string in a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** manage to find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so the string will point to a wrong location; - somehow ASAN realizes that the string points to a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** manage to find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so the string will point to a wrong location; - somehow sometimes ASAN realizes that the string points to a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. Following to the proper stack shared in comment 5, I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** manage to find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so the string will point to a wrong location; - somehow sometimes ASAN realizes that the string points to a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```
Hello, Sorry I am a bit late to the party. Following the proper stack shared in comment 5, I have looked at the conditions under which `LdrpFindLoadedDllByMappingFile` gets called, and I do not think this is a race condition. I think the problem is as follows: - `LdrpFindLoadedDllByMappingFile` seems to be called when we do **not** manage to find a module just based on the DLL name directly, but the path we use in `GetModuleHandle` can be resolved to a file on disk; - in that case, `LdrpFindLoadedDllByMappingFile` will call `NtMapViewOfSection` to map the DLL file and somehow try to find the module based on that; - however the file is mapped as raw bytes, resulting in a different memory layout from the one we would have if we were truly loading the library; - but our (outdated) chromium sandbox code identifies this call to `NtMapViewOfSection` as if we were loading a library; - it will [resolve the DLL name found in the export directory as an RVA](https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#458), but this is wrong since the file is mapped as raw bytes, so the string will point to a wrong location; - somehow sometimes ASAN realizes that the string points to a wrong location. I believe the proper fix is to update our implementation for `IsValidImageSection` based on the changes that chromium people have integrated themselves. We currently have diverged: https://searchfox.org/mozilla-central/source/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc#379-408 https://source.chromium.org/chromium/chromium/src/+/main:sandbox/win/src/sandbox_nt_util.cc;l=377 In particular, there is a comment that states: ``` // Windows 10 2009+ may open PEs as SEC_IMAGE_NO_EXECUTE in non-dll-loading // paths which looks identical to dll-loading unless we check if the section // handle has execute rights. ```