New issue
Advanced search Search tips

Issue 905137 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 15
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

usa-after-poison in parkable_string.cc

Project Member Reported by mpdenton@google.com, Nov 14

Issue description

It appears that after |CompressInBackground| unpoisons a string's memory, someone could call |ToString| and then |Unlock|, which unpoisons and then poisons the string. Then when |GzipCompress| actually uses the bytes, I get a use-after-poison. Presumably this isn't a real bug, just an ASAN poisoning bug, but it makes it a little difficult to test for other bugs with ASAN.

To reproduce this you can just build Chrome with ASAN, open a bunch of tabs and Google things in those tabs, the report appears pretty quickly.
 
Status: Assigned (was: Untriaged)
Thanks for the report!
Uploaded https://chromium-review.googlesource.com/c/chromium/src/+/1335585 to fix it, sorry about the issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 15

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

commit 3cb93d0aa21a7cb92b92ca169104e5efab7c295c
Author: Benoît Lizé <lizeb@chromium.org>
Date: Thu Nov 15 07:38:57 2018

blink/bindings: Fix false-positive ASAN warning in ParkableString.

The following sequence is racy with the current ASAN checks in ParkableString,
on the main thread:

Park()
Lock()
ToString()
Unlock()

Park() poisons the string, ToString() unpoisons it, and Unlock() poisons it
again. If this last call happens while the compression is in progress, then this
is a use-after-poison.

This is not a real issue, merely an overaly eager poisoning, still making using
ASAN builds painful. Fix it by making sure the string stays unpoisoned during
compression.
Also adds a regression test.

Bug:  905137 ,877044
Change-Id: I5276b9ae6eee4abe2f2bf041818d1ba17358a80a
Reviewed-on: https://chromium-review.googlesource.com/c/1335585
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608289}
[modify] https://crrev.com/3cb93d0aa21a7cb92b92ca169104e5efab7c295c/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/3cb93d0aa21a7cb92b92ca169104e5efab7c295c/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment