New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 800352 link

Starred by 2 users
Status: Fixed
Owner:
█ zhaobin@chromium.org
Last visit > 30 days ago
Closed: Jan 2018
Cc:
siggi@chromium.org
w...@chromium.org
█ imch...@chromium.org
mfo...@chromium.org
taku...@chromium.org
powerb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Use after free of media_router::CastMediaSinkServiceImpl

Project Member Reported by chrisha@chromium.org, Jan 9 2018 
Chrome Version: 65.0.3303.3
OS: Windows

This crash is seen on recent ASAN releases. For an example report see go/crash/a2509fdc07ffa837

Stacks included inline for convenience:

Use after free stack:
	0x5d97efc1	(chrome.dll -ref_counted.h:170 )	base::subtle::RefCountedThreadSafeBase::AddRef()
0x5d9c2662	(chrome.dll -weak_ptr.cc:72 )	base::internal::WeakPtrBase::WeakPtrBase(base::internal::WeakReference const &,unsigned int)
0x5eb049c4	(chrome.dll -weak_ptr.h:313 )	base::WeakPtrFactory<device::U2fHidDiscovery>::GetWeakPtr()
0x5fc12577	(chrome.dll -cast_media_sink_service_impl.cc:243 )	media_router::CastMediaSinkServiceImpl::Start()
0x5da90685	(chrome.dll -task_annotator.cc:55 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *)
0x5d9dc594	(chrome.dll -message_loop.cc:391 )	base::MessageLoop::RunTask(base::PendingTask *)
0x5d9dc905	(chrome.dll -message_loop.cc:403 )	base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
0x5d9dce1a	(chrome.dll -message_loop.cc:447 )	base::MessageLoop::DoWork()
0x5da6de88	(chrome.dll -message_pump_win.cc:475 )	base::MessagePumpForIO::DoRunLoop()
0x5da6ca80	(chrome.dll -message_pump_win.cc:56 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x5d9b9e02	(chrome.dll -run_loop.cc:130 )	base::RunLoop::Run()
0x5d9e5bff	(chrome.dll -thread.cc:255 )	base::Thread::Run(base::RunLoop *)
0x5e67b685	(chrome.dll -browser_thread_impl.cc:248 )	content::BrowserThreadImpl::IOThreadRun(base::RunLoop *)
0x5e67c425	(chrome.dll -browser_thread_impl.cc:283 )	content::BrowserThreadImpl::Run(base::RunLoop *)
0x5d9e638f	(chrome.dll -thread.cc:338 )	base::Thread::ThreadMain()
0x5d9cdfcd	(chrome.dll -platform_thread_win.cc:89 )	base::`anonymous namespace'::ThreadFunc
0x77403369	(kernel32.dll + 0x00013369 )	BaseThreadInitThunk
0x77d298f1	(ntdll.dll + 0x000398f1 )	__RtlUserThreadStart
0x77d298c4	(ntdll.dll + 0x000398c4 )	_RtlUserThreadStart

ASAN Free Stack Trace:
	0x68a2b83b	(syzyasan_rtl.dll -block_heap_manager.cc:315 )	agent::asan::heap_managers::BlockHeapManager::Free(unsigned int,void *)
0x68a2424d	(syzyasan_rtl.dll -rtl_impl.cc:124 )	asan_HeapFree
0x5da5c3bb	(chrome.dll -allocator_shim_default_dispatch_to_winheap.cc:55 )	`anonymous namespace'::DefaultWinHeapFreeImpl
0x5d987932	(chrome.dll -allocator_shim_override_ucrt_symbols_win.h:55 )	free
0x5d9c2525	(chrome.dll -weak_ptr.cc:55 )	base::internal::WeakReferenceOwner::GetRef()
0x5eb049bb	(chrome.dll -weak_ptr.h:313 )	base::WeakPtrFactory<device::U2fHidDiscovery>::GetWeakPtr()
0x5fc10408	(chrome.dll -cast_media_sink_service_impl.cc:608 )	media_router::CastMediaSinkServiceImpl::GetDialSinkAddedCallback()
0x5fc0864e	(chrome.dll -cast_media_sink_service.cc:210 )	media_router::CastMediaSinkService::GetDialSinkAddedCallback()
0x5fbec552	(chrome.dll -media_router_desktop.cc:175 )	media_router::MediaRouterDesktop::StartDiscovery()
0x5fbec0f8	(chrome.dll -media_router_desktop.cc:122 )	media_router::MediaRouterDesktop::RegisterExtensionMediaRouteProvider(mojo::InterfacePtr<media_router::mojom::MediaRouteProvider>)
0x5fbec220	(chrome.dll -media_router_desktop.cc:105 )	media_router::MediaRouterDesktop::RegisterMediaRouteProvider(media_router::MediaRouteProviderId,mojo::InterfacePtr<media_router::mojom::MediaRouteProvider>,base::OnceCallback<void >)
0x5ed0d9ee	(chrome.dll -media_router.mojom.cc:4622 )	media_router::mojom::MediaRouterStubDispatch::AcceptWithResponder(media_router::mojom::MediaRouter *,mojo::Message *,std::unique_ptr<mojo::MessageReceiverWithStatus,std::default_delete<mojo::MessageReceiverWithStatus> >)
0x5fbf699a	(chrome.dll -media_router.mojom.h:472 )	media_router::mojom::MediaRouterStub<mojo::RawPtrImplRefTraits<media_router::mojom::MediaRouter> >::AcceptWithResponder(mojo::Message *,std::unique_ptr<mojo::MessageReceiverWithStatus,std::default_delete<mojo::MessageReceiverWithStatus> >)
0x5ef56c32	(chrome.dll -interface_endpoint_client.cc:394 )	mojo::InterfaceEndpointClient::HandleValidatedMessage(mojo::Message *)
0x5ef5a07e	(chrome.dll -filter_chain.cc:41 )	mojo::FilterChain::Accept(mojo::Message *)
0x5ef56a27	(chrome.dll -interface_endpoint_client.cc:306 )	mojo::InterfaceEndpointClient::HandleIncomingMessage(mojo::Message *)
0x5ef510f2	(chrome.dll -multiplex_router.cc:885 )	mojo::internal::MultiplexRouter::ProcessIncomingMessage(mojo::internal::MultiplexRouter::MessageWrapper *,mojo::internal::MultiplexRouter::ClientCallBehavior,base::SequencedTaskRunner *)
0x5ef4f064	(chrome.dll -multiplex_router.cc:604 )	mojo::internal::MultiplexRouter::Accept(mojo::Message *)
0x5ef5a07e	(chrome.dll -filter_chain.cc:41 )	mojo::FilterChain::Accept(mojo::Message *)
0x5ef4c706	(chrome.dll -connector.cc:444 )	mojo::Connector::ReadSingleMessage(unsigned int *)
0x5ef4c377	(chrome.dll -connector.cc:474 )	mojo::Connector::ReadAllAvailableMessages()
0x5ef4bf42	(chrome.dll -connector.cc:377 )	mojo::Connector::OnHandleReadyInternal(unsigned int)
0x5fd5648a	(chrome.dll -bind_internal.h:353 )	base::internal::Invoker<base::internal::BindState<void (__thiscall `anonymous namespace'::FlagsDOMHandler::*)(base::ListValue const *),base::internal::UnretainedWrapper<`anonymous namespace'::FlagsDOMHandler> >,void __cdecl(base::ListValue const *)>::Run
0x5f1f66dd	(chrome.dll -file_stream_context.cc:27 )	net::`anonymous namespace'::CallInt64ToInt
0x601a4067	(chrome.dll -bind_internal.h:350 )	base::internal::Invoker<base::internal::BindState<void (*)(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,PermissionRequestCreator *,base::RepeatingCallback<void > const &),std::basic_string<char,std::char_traits<char>,std::allocator<char> > >,void >::Run(base::internal::BindStateBase *,PermissionRequestCreator *,base::RepeatingCallback<void > const &)
0x5ef62bd0	(chrome.dll -simple_watcher.cc:276 )	mojo::SimpleWatcher::OnHandleReady(int,unsigned int,mojo::HandleSignalsState const &)
0x5f9ceffe	(chrome.dll -bind_internal.h:314 )	base::internal::InvokeHelper<1,void>::MakeItSo<void ( storage::LocalFileStreamWriter::*const &)(net::IOBuffer *,int,base::RepeatingCallback<void > const &),base::WeakPtr<storage::LocalFileStreamWriter> const &,net::IOBuffer *,int const &,base::RepeatingCallback<void > const &>(void ( storage::LocalFileStreamWriter::*const &)(net::IOBuffer *,int,base::RepeatingCallback<void > const &),base::WeakPtr<storage::LocalFileStreamWriter> const &,net::IOBuffer * &&,int const &,base::RepeatingCallback<void > const &)
0x5ef61fd6	(chrome.dll -bind_internal.h:368 )	base::internal::Invoker<base::internal::BindState<void ( mojo::SimpleWatcher::*)(int,unsigned int,mojo::HandleSignalsState const &),base::WeakPtr<mojo::SimpleWatcher>,int,unsigned int,mojo::HandleSignalsState>,void >::RunImpl<void ( mojo::SimpleWatcher::*const &)(int,unsigned int,mojo::HandleSignalsState const &),std::tuple<base::WeakPtr<mojo::SimpleWatcher>,int,unsigned int,mojo::HandleSignalsState> const &,0,1,2,3>(void ( mojo::SimpleWatcher::*const &)(int,unsigned int,mojo::HandleSignalsState const &),std::tuple<base::WeakPtr<mojo::SimpleWatcher>,int,unsigned int,mojo::HandleSignalsState> const &,std::integer_sequence<unsigned int,0,1,2,3>)
0x5ef62c69	(chrome.dll -bind_internal.h:350 )	base::internal::Invoker<base::internal::BindState<void ( mojo::SimpleWatcher::*)(int,unsigned int,mojo::HandleSignalsState const &),base::WeakPtr<mojo::SimpleWatcher>,int,unsigned int,mojo::HandleSignalsState>,void >::Run(base::internal::BindStateBase *)
0x5da90686	(chrome.dll -task_annotator.cc:55 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *)
0x5d9dc595	(chrome.dll -message_loop.cc:392 )	base::MessageLoop::RunTask(base::PendingTask *)
0x5d9dc906	(chrome.dll -message_loop.cc:403 )	base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
0x5d9dce1b	(chrome.dll -message_loop.cc:447 )	base::MessageLoop::DoWork()
0x5da6cfbe	(chrome.dll -message_pump_win.cc:174 )	base::MessagePumpForUI::DoRunLoop()
0x5da6ca81	(chrome.dll -message_pump_win.cc:58 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x5d9b9e03	(chrome.dll -run_loop.cc:133 )	base::RunLoop::Run()
0x5eecb989	(chrome.dll -chrome_browser_main.cc:1959 )	ChromeBrowserMainParts::MainMessageLoopRun(int *)
0x5e672d59	(chrome.dll -browser_main_loop.cc:1196 )	content::BrowserMainLoop::RunMainMessageLoopParts()
0x5e66e1ea	(chrome.dll -browser_main.cc:46 )	content::BrowserMain(content::MainFunctionParams const &)
0x5edf4072	(chrome.dll -content_main_runner.cc:427 )	content::RunNamedProcessTypeMain(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,content::MainFunctionParams const &,content::ContentMainDelegate *)
0x5edf3f65	(chrome.dll -content_main_runner.cc:710 )	content::ContentMainRunnerImpl::Run()
0x5ee139cc	(chrome.dll -main.cc:456 )	service_manager::Main(service_manager::MainParams const &)
0x5edf366e	(chrome.dll -content_main.cc:19 )	content::ContentMain(content::ContentMainParams const &)
0x5e190704	(chrome.dll -chrome_main.cc:128 )	ChromeMain
0x00f8e8ce	(chrome.exe -main_dll_loader_win.cc:201 )	MainDllLoader::Launch(HINSTANCE__ *,base::TimeTicks)
0x00f8daa2	(chrome.exe -chrome_exe_main_win.cc:231 )	wWinMain
0x00fb5f28	(chrome.exe -exe_common.inl:283 )	__scrt_common_main_seh
0x7740336a	(kernel32.dll + 0x0001336a )	BaseThreadInitThunk
0x77d298f2	(ntdll.dll + 0x000398f2 )	__RtlUserThreadStart
0x77d298c5	(ntdll.dll + 0x000398c5 )	_RtlUserThreadStart

Allocation stack trace:
	0x68a2b579	(syzyasan_rtl.dll -block_heap_manager.cc:211 )	agent::asan::heap_managers::BlockHeapManager::Allocate(unsigned int,unsigned int)
0x68a241a3	(syzyasan_rtl.dll -rtl_impl.cc:103 )	asan_HeapAlloc
0x5da5c2ea	(chrome.dll -allocator_shim_default_dispatch_to_winheap.cc:18 )	`anonymous namespace'::DefaultWinHeapMallocImpl
0x5d9878ea	(chrome.dll -allocator_shim_override_ucrt_symbols_win.h:51 )	malloc
0x602bf8f8	(chrome.dll -new_scalar.cpp:19 )	operator new(unsigned int)
0x5d9c2487	(chrome.dll -weak_ptr.cc:55 )	base::internal::WeakReferenceOwner::GetRef()
0x5eb049bb	(chrome.dll -weak_ptr.h:313 )	base::WeakPtrFactory<device::U2fHidDiscovery>::GetWeakPtr()
0x5fc12578	(chrome.dll -cast_media_sink_service_impl.cc:243 )	media_router::CastMediaSinkServiceImpl::Start()
0x5da90686	(chrome.dll -task_annotator.cc:55 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *)
0x5d9dc595	(chrome.dll -message_loop.cc:392 )	base::MessageLoop::RunTask(base::PendingTask *)
0x5d9dc906	(chrome.dll -message_loop.cc:403 )	base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
0x5d9dce1b	(chrome.dll -message_loop.cc:447 )	base::MessageLoop::DoWork()
0x5da6de89	(chrome.dll -message_pump_win.cc:475 )	base::MessagePumpForIO::DoRunLoop()
0x5da6ca81	(chrome.dll -message_pump_win.cc:58 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x5d9b9e03	(chrome.dll -run_loop.cc:133 )	base::RunLoop::Run()
0x5d9e5c00	(chrome.dll -thread.cc:256 )	base::Thread::Run(base::RunLoop *)
0x5e67b686	(chrome.dll -browser_thread_impl.cc:249 )	content::BrowserThreadImpl::IOThreadRun(base::RunLoop *)
0x5e67c426	(chrome.dll -browser_thread_impl.cc:283 )	content::BrowserThreadImpl::Run(base::RunLoop *)
0x5d9e6390	(chrome.dll -thread.cc:338 )	base::Thread::ThreadMain()
0x5d9cdfce	(chrome.dll -platform_thread_win.cc:91 )	base::`anonymous namespace'::ThreadFunc
0x7740336a	(kernel32.dll + 0x0001336a )	BaseThreadInitThunk
0x77d298f2	(ntdll.dll + 0x000398f2 )	__RtlUserThreadStart
0x77d298c5	(ntdll.dll + 0x000398c5 )	_RtlUserThreadStart

Comment 1 by w...@chromium.org, Jan 9 2018 

The stacks indicate UAF of the internal Flag object, and the free stack
indicates that that is happening when all the references to it have gone,
and the called invokes GetWeakPtr again. Since the Flag is RefCounted,
that's really weird!

Chris, are you suspecting that this is actually due to the
CastMediaSinkServiceImpl having already been deleted?

Comment 2 by chrisha@chromium.org, Jan 9 2018 

Sorry, I was just mechanically filing issues to clear a backlog, and I wasn't digging too deep.

Poking at this a bit now, what you're describing is indeed what appears to be happening, which is indeed non-sensical. Given that this has occurred only once it's not out of the realm of possibility of this being due to a magical errant bit flip (which we have seen before). Wait to see if any other crashes occur?

Comment 3 by siggi@chromium.org, Jan 9 2018

Labels: -Pri-3 Pri-2
This looks like a race, as the free is happening on the UI thread, whereas the alloc and crash are on a worker thread.
I'm guessing this is due to abusing the weak pointer factory, perhaps because a mojo interface has been incorrectly bound to the UI thread.

Comment 4 by siggi@chromium.org, Jan 9 2018

Cc: w...@chromium.org siggi@chromium.org

Comment 5 by zhaobin@chromium.org, Jan 9 2018 

It is weird that CMSSImpl::Start() invokes device::U2fHidDiscovery's WeakPtrFactory. It should invoke media_router::CastMediaSinkServiceImpl's WeakPtrFactory.

It seems that https://chromium-review.googlesource.com/c/chromium/src/+/821490 should fix this. Please let us know if there are more crashes.

Comment 6 by w...@chromium.org, Jan 9 2018

Cc: imch...@chromium.org
Labels: M-65
Status: Fixed (was: Untriaged)
Re #5: When symbolizing stack frames in templates, they will often be mis-attributed if the template happens to be re-used, e.g. in this case the code for base::WeakPtrFactory<T>::GetWeakPtr() is identical across all of the specializations of WeakPtrFactory, so when we symbolize the value of T is pretty random.

IIUC what's happened here is that:
1. On the IO thread GetWeakPtr() is called, allocates the Flag and assigns it to the |flag_| member. At this point it has ref-count 1.
2. On the UI thread GetWeakPtr() is called, and sees that the current Flag has only a single ref-count, i.e. there are no outstanding WeakPtrs, so it re-creates a new Flag, destroying the old.
3. The IO thread is scheduled again and returns the original Flag - the caller wraps it in a WeakPtr which AddRef's it, but since it was already freed, we UAF.

I agree, it looks like the revised GetDialSinkAddedCallback() addresses this WeakPtr misuse (see https://cs.chromium.org/chromium/src/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc?sq=package:chromium&l=608).
►

Sign in to add a comment