Skip to content

Conversation

TaWeiTu
Copy link
Contributor

@TaWeiTu TaWeiTu commented Sep 10, 2021

There was a bug found by the fuzzer where we might access wire_writer_ before
finishing SETUP_TRANSPORT (and thus constructing wire_writer_). This PR
fixes such issue by making sure that we won't proceed with any requests
until the connection is fully established.

Since binder transactions may be coming from multiple different threads,
this PRs guard some of the WireReaderImpl's member with a mutex to make
sure there's no races between threads.

@TaWeiTu TaWeiTu added the release notes: no Indicates if PR should not be in release notes label Sep 10, 2021
@TaWeiTu TaWeiTu requested a review from sifmelcara September 10, 2021 09:21
@TaWeiTu TaWeiTu force-pushed the fix-wire-reader-race branch from df5eb9c to 1724178 Compare September 11, 2021 01:32
@TaWeiTu
Copy link
Contributor Author

TaWeiTu commented Sep 11, 2021

Modified tests slightly to make them work. @sifmelcara would you mind taking another look before I merge this?

@TaWeiTu TaWeiTu force-pushed the fix-wire-reader-race branch from 1724178 to aac443b Compare September 11, 2021 01:56
@sifmelcara
Copy link
Contributor

LGTM

@TaWeiTu TaWeiTu force-pushed the fix-wire-reader-race branch 2 times, most recently from 1d2ff5e to f639c93 Compare September 11, 2021 03:47
There was a bug found by the fuzzer where we might access wire_writer_ before
finishing SETUP_TRANSPORT (and thus constructing wire_writer_). This PR
fixes such issue by making sure that we won't proceed with any requests
until the connection is fully established.

Since binder transactions may be coming from multiple different threads,
this PRs guard some of the WireReaderImpl's member with a mutex to make
sure there's no races between threads.
@TaWeiTu TaWeiTu force-pushed the fix-wire-reader-race branch from f639c93 to 2fbff87 Compare September 11, 2021 04:30
@TaWeiTu TaWeiTu enabled auto-merge (squash) September 11, 2021 05:00
@TaWeiTu TaWeiTu merged commit 554bbb6 into grpc:master Sep 11, 2021
lidizheng pushed a commit to lidizheng/grpc that referenced this pull request Sep 23, 2021
There was a bug found by the fuzzer where we might access wire_writer_ before
finishing SETUP_TRANSPORT (and thus constructing wire_writer_). This PR
fixes such issue by making sure that we won't proceed with any requests
until the connection is fully established.

Since binder transactions may be coming from multiple different threads,
this PRs guard some of the WireReaderImpl's member with a mutex to make
sure there's no races between threads.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/core release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants