New issue
Advanced search Search tips

Issue 909720 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 30
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug

Blocking:
issue 896019



Sign in to add a comment

Crash in ntdll!RtlSizeHeap with GWP-ASan enabled

Project Member Reported by vtsyrklevich@chromium.org, Nov 28

Issue description

With a recent Chrome build with the GWP-ASan feature flag CLI, Chrome crashes after it's been left idle for a while. The crash happens in ntdll.dll!RtlSizeHeap either by accessing just underneath a GWP-ASan allocation (offset 0xfff into the preceding page) or by accessing the invalid pointer 0x24 (I believe this happens for right-aligned allocations.) The crash appears to be happening on a call to malloc() from sqlite3MemMalloc().

I'm still investigating root cause.
 
sqlite3 uses _msize() which we don't intercept (there are currently no windows GetSizeEstimate hooks), so it's possible to reach RtlSizeHeap() with a GWP-ASan allocation causing an exception. Instrumenting _msize() seems to fix the issue.
Cc: kcc@chromium.org
Blocking: 896019
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f379f24ee452fc163a52c2f573e4a250c3b38d2

commit 4f379f24ee452fc163a52c2f573e4a250c3b38d2
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Nov 29 05:20:05 2018

allocator shim: Add windows _msize interception

Currently, the Windows allocator shims do not hook _msize() causing
sporadic crashes when GWP-ASan is enabled. This occurs because sqlite3
uses _msize() and the native implementation does not properly handle
allocations not returned by the native allocator.

Furthermore, the current (unused) implementation of
WinHeapGetSizeEstimate() seems to be an artifact of the original
implementation from crrev.com/2163783003. It incorrectly increases the
size estimate returned by the native allocator, which can cause
exceptions in sqlite3 (there are routines that will read the _msize of
an allocation and assume the returned size forms a safe bound.) I
changed this routine to just return the allocation size returned by the
native allocator.

Bug:  909720 
Change-Id: Ie8975053992161cdc3e447f75733345f0a142978
Reviewed-on: https://chromium-review.googlesource.com/c/1354219
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612051}
[modify] https://crrev.com/4f379f24ee452fc163a52c2f573e4a250c3b38d2/base/allocator/allocator_shim_override_ucrt_symbols_win.h
[modify] https://crrev.com/4f379f24ee452fc163a52c2f573e4a250c3b38d2/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/4f379f24ee452fc163a52c2f573e4a250c3b38d2/base/allocator/winheap_stubs_win.cc

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09bb5f10ed625caa1f375374ff0004b6cf2c906f

commit 09bb5f10ed625caa1f375374ff0004b6cf2c906f
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Sat Dec 01 00:56:43 2018

GWP-ASan: Add GetSizeEstimate test

Bug:  909720 
Change-Id: Ie88f31f58de2094583722884bb25fbbbee8a85ee
Reviewed-on: https://chromium-review.googlesource.com/c/1357575
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612881}
[modify] https://crrev.com/09bb5f10ed625caa1f375374ff0004b6cf2c906f/components/gwp_asan/client/sampling_allocator_shims_unittest.cc

Sign in to add a comment