New issue
Advanced search Search tips

Issue 848373 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

UaF in PrerenderBrowserTest.FirstContentfulPaintTimingNoCommit test

Project Member Reported by lfg@chromium.org, May 31 2018

Issue description

https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_asan_rel_ng/23966
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_asan_rel_ng/23970
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/linux_chromium_asan_rel_ng/23936

[ RUN      ] PrerenderBrowserTest.FirstContentfulPaintTimingNoCommit
Xlib:  extension "RANDR" missing on display ":99".
[3384:3492:0531/103459.950859:ERROR:bus.cc(394)] Failed to connect to the bus: Could not parse server address: Unknown address type (examples of valid types are "tcp" and on UNIX "unix")
[3384:3384:0531/103500.038544:WARNING:password_store_factory.cc(250)] Using basic (unencrypted) store for password storage. See https://chromium.googlesource.com/chromium/src/+/master/docs/linux_password_storage.md for more information about password storage options.
(browser_tests:3384): LIBDBUSMENU-GLIB-WARNING **: Unable to get session bus: Unknown or unsupported transport 'disabled' for address 'disabled:'
[3384:3384:0531/103500.849192:WARNING:gaia_auth_fetcher.cc(902)] Could not reach Google Accounts servers: errno -11
[3384:3447:0531/103501.690440:ERROR:test_database_manager.cc(64)] Not implemented reached in virtual bool safe_browsing::TestSafeBrowsingDatabaseManager::CheckResourceUrl(const GURL &, safe_browsing::SafeBrowsingDatabaseManager::Client *)
[3384:3384:0531/103501.752742:WARNING:gaia_auth_fetcher.cc(902)] Could not reach Google Accounts servers: errno -102
[3384:3549:0531/103502.061521:WARNING:embedded_test_server.cc(229)] Request not handled. Returning 404: /favicon.ico
=================================================================
==3384==ERROR: AddressSanitizer: stack-use-after-return on address 0x7fc2dbd143e0 at pc 0x000017e3bdc4 bp 0x7ffda473cbb0 sp 0x7ffda473cba8
READ of size 8 at 0x7fc2dbd143e0 thread T0 (browser_tests)
    #0 0x17e3bdc3 in GetCurrentTimeTicks chrome/browser/prerender/prerender_manager.cc:993:23
    #1 0x17e3bdc3 in prerender::PrerenderManager::CleanUpOldNavigations(std::__1::vector<prerender::PrerenderManager::NavigationRecord, std::__1::allocator<prerender::PrerenderManager::NavigationRecord> >*, base::TimeDelta) chrome/browser/prerender/prerender_manager.cc:1109
    #2 0x17e39cc3 in GetPrefetchInformation chrome/browser/prerender/prerender_manager.cc:1060:3
    #3 0x17e39cc3 in prerender::PrerenderManager::RecordNoStateFirstContentfulPaint(GURL const&, bool, bool, base::TimeDelta) chrome/browser/prerender/prerender_manager.cc:496
    #4 0x17c88521 in NoStatePrefetchPageLoadMetricsObserver::OnFirstContentfulPaintInPage(page_load_metrics::mojom::PageLoadTiming const&, page_load_metrics::PageLoadExtraInfo const&) chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.cc:52:23
    #5 0x17cdbb6c in DispatchObserverTimingCallbacks chrome/browser/page_load_metrics/page_load_tracker.cc:150:15
    #6 0x17cdbb6c in page_load_metrics::PageLoadTracker::OnTimingChanged() chrome/browser/page_load_metrics/page_load_tracker.cc:600
    #7 0x17cbcabf in page_load_metrics::PageLoadMetricsUpdateDispatcher::DispatchTimingUpdates() chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc:581:12
    #8 0x17cba7fa in page_load_metrics::PageLoadMetricsUpdateDispatcher::ShutDown() chrome/browser/page_load_metrics/page_load_metrics_update_dispatcher.cc:385:5
    #9 0x17cd07a8 in page_load_metrics::PageLoadTracker::~PageLoadTracker() chrome/browser/page_load_metrics/page_load_tracker.cc:215:30
    #10 0x17cd34dc in page_load_metrics::PageLoadTracker::~PageLoadTracker() chrome/browser/page_load_metrics/page_load_tracker.cc:207:37
    #11 0x17bfb435 in operator() buildtools/third_party/libc++/trunk/include/memory:2321:5
    #12 0x17bfb435 in reset buildtools/third_party/libc++/trunk/include/memory:2634
    #13 0x17bfb435 in operator= buildtools/third_party/libc++/trunk/include/memory:2592
    #14 0x17bfb435 in page_load_metrics::MetricsWebContentsObserver::WebContentsDestroyed() chrome/browser/page_load_metrics/metrics_web_contents_observer.cc:114
    #15 0x12104054 in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:649:14
    #16 0x12106bdc in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:550:37
    #17 0x229c2e77 in operator() buildtools/third_party/libc++/trunk/include/memory:2321:5
    #18 0x229c2e77 in reset buildtools/third_party/libc++/trunk/include/memory:2634
    #19 0x229c2e77 in TabStripModel::SendDetachWebContentsNotifications(TabStripModel::DetachNotifications*) chrome/browser/ui/tabs/tab_strip_model.cc:460
    #20 0x229d534e in TabStripModel::CloseWebContentses(base::span<content::WebContents* const, 18446744073709551615ul>, unsigned int) chrome/browser/ui/tabs/tab_strip_model.cc:1366:3
    #21 0x229c7fd9 in TabStripModel::InternalCloseTabs(base::span<content::WebContents* const, 18446744073709551615ul>, unsigned int) chrome/browser/ui/tabs/tab_strip_model.cc:1284:27
    #22 0x229c7613 in TabStripModel::CloseAllTabs() chrome/browser/ui/tabs/tab_strip_model.cc:582:3
    #23 0x22ff9b16 in BrowserView::CanClose() chrome/browser/ui/views/frame/browser_view.cc:1970:15
    #24 0x1ab49bc6 in views::Widget::Close() ui/views/widget/widget.cc:577:46
    #25 0x181195be in BrowserCloseManager::CloseBrowsers() chrome/browser/lifetime/browser_close_manager.cc:170:24
    #26 0x1811a383 in BrowserCloseManager::CheckForDownloadsInProgress() chrome/browser/lifetime/browser_close_manager.cc:107:5
    #27 0x18119faa in BrowserCloseManager::TryToCloseBrowsers() chrome/browser/lifetime/browser_close_manager.cc:82:3
    #28 0x17a82805 in chrome::CloseAllBrowsers() chrome/browser/lifetime/application_lifetime.cc:191:26
    #29 0x17a8396c in AttemptExitInternal chrome/browser/lifetime/application_lifetime.cc:147:39
    #30 0x17a8396c in chrome::AttemptExit() chrome/browser/lifetime/application_lifetime.cc:294
    #31 0x17232d06 in Run base/callback.h:96:12
    #32 0x17232d06 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:101
    #33 0x172d24a8 in base::internal::IncomingTaskQueue::RunTask(base::PendingTask*) base/message_loop/incoming_task_queue.cc:124:19
    #34 0x172cb020 in base::MessageLoop::RunTask(base::PendingTask*) base/message_loop/message_loop.cc:319:25
    #35 0x172cbcbf in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) base/message_loop/message_loop.cc:329:5
    #36 0x172cc520 in base::MessageLoop::DoWork() base/message_loop/message_loop.cc:373:16
    #37 0x172e17d5 in HandleDispatch base/message_loop/message_pump_glib.cc:263:25
    #38 0x172e17d5 in base::(anonymous namespace)::WorkSourceDispatch(_GSource*, int (*)(void*), void*) base/message_loop/message_pump_glib.cc:109
    #39 0x7fc2e976fe03 in g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x48e03)
Address 0x7fc2dbd143e0 is located in stack of thread T0 (browser_tests) at offset 992 in frame
    #0 0x1d5e6eaf in cc::Layer::InsertChild(scoped_refptr<cc::Layer>, unsigned long) cc/layers/layer.cc:218
  This frame has 8 object(s):
    [32, 328) 'ref.tmp.i72'
    [400, 696) 'ref.tmp.i62' (line 206)
    [768, 1064) 'ref.tmp.i55' <== Memory access at offset 992 is inside this variable
    [1136, 1432) 'ref.tmp.i48'
    [1504, 1800) 'ref.tmp.i41'
    [1872, 2168) 'ref.tmp.i34' (line 232)
    [2240, 2536) 'ref.tmp.i'
    [2608, 2904) 'ref.tmp' (line 219)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-return chrome/browser/prerender/prerender_manager.cc:993:23 in GetCurrentTimeTicks
Shadow bytes around the buggy address:
  0x0ff8db79a820: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a830: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a840: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a850: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a860: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
=>0x0ff8db79a870: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5[f5]f5 f5 f5
  0x0ff8db79a880: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a890: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a8a0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a8b0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  0x0ff8db79a8c0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc

 
 Issue 848552  has been merged into this issue.
Labels: Sheriff-Chromium

Comment 3 by w...@chromium.org, Jun 1 2018

Cc: w...@chromium.org
Labels: M-69
Owner: tzik@chromium.org
Status: Assigned (was: Untriaged)
This was regressed by https://chromium-review.googlesource.com/c/chromium/src/+/989457 I think - the CL places a SimpleTestTickClock in the test-body stack frame, meaning that it may be torn-down before all Browser instances are closed by the fixture teardown code.  Moving the SimpleTestTickClock to be a test class member should resolve the issue.

Comment 4 by w...@chromium.org, Jun 1 2018

Cc: tzik@chromium.org
Owner: w...@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 1 2018

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

commit 1636332a095f00b78684b004e68028737e61de44
Author: Wez <wez@chromium.org>
Date: Fri Jun 01 20:55:32 2018

Fix potential use-after-free in PrerenderBrowserTest tests.

Tests were creating SimpleTestTickClocks in the test body, causing a
race between the clock and Browser instances being torn-down, leading
to a potential use-after-free in the tests.

TBR=tzik

Bug: 789079,  848373 
Change-Id: Iebc319999915c0794063b0c410bc335908135b85
Reviewed-on: https://chromium-review.googlesource.com/1082961
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563790}
[modify] https://crrev.com/1636332a095f00b78684b004e68028737e61de44/chrome/browser/prerender/prerender_browsertest.cc

Comment 6 by w...@chromium.org, Jun 1 2018

Status: Fixed (was: Started)

Sign in to add a comment