Skip to content

Commit

Permalink
Revert "Only pass executable sections to OnDllLoaded for patching"
Browse files Browse the repository at this point in the history
This reverts commit df9e203.

Reason for revert: Suspected culprit of tree closer on win-asan: https://ci.chromium.org/p/chromium/builders/ci/win32-archive-rel/17873?

Original change's description:
> Only pass executable sections to OnDllLoaded for patching
>
> On recent Windows insider builds GetModuleHandleExW loads sections for
> inspection as non-executable images, rather than as files. This leads
> to our hooks detecting the SEC_IMAGE attribute and potentially patching
> functions (e.g. for user32.dll).
>
> This caused content_browsertests to fail as it pinned user32.dll in some
> processes. With this change, the tests run again.
>
> See crbug.com/1143397 for a full discussion.
>
> Bug: 1143397
> Change-Id: I3b75464d0442160a417e4cb7084306841aaf76f7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511531
> Reviewed-by: Will Harris <wfh@chromium.org>
> Commit-Queue: Alex Gough <ajgo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#823354}

TBR=wfh@chromium.org,ajgo@chromium.org

Change-Id: Ia164ea218daf7771f025dfa57a3a4bf25c41eac2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1143397
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515501
Reviewed-by: Meredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823372}
  • Loading branch information
Meredith Lane authored and Commit Bot committed Nov 3, 2020
1 parent eeda0da commit 71b5f24
Showing 1 changed file with 0 additions and 14 deletions.
14 changes: 0 additions & 14 deletions sandbox/win/src/sandbox_nt_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -404,20 +404,6 @@ bool IsValidImageSection(HANDLE section,
if (!(basic_info.Attributes & SEC_IMAGE))
return false;

// 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.
OBJECT_BASIC_INFORMATION obj_info;
ULONG obj_size_returned;
ret = g_nt.QueryObject(section, ObjectBasicInformation, &obj_info,
sizeof(obj_info), &obj_size_returned);

if (!NT_SUCCESS(ret) || sizeof(obj_info) != obj_size_returned)
return false;

if (!(obj_info.GrantedAccess & SECTION_MAP_EXECUTE))
return false;

return true;
}

Expand Down

0 comments on commit 71b5f24

Please sign in to comment.