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

Issue 811477 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Data race in sandbox::ProxyLocaltimeCallToBrowser

Project Member Reported by ClusterFuzz, Feb 12 2018

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5766392054546432

Fuzzer: ochang_domfuzzer
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race WRITE 1
Crash Address: 0x7f4ddffddcc0
Crash State:
  sandbox::ProxyLocaltimeCallToBrowser
  localtime
  usrsctp_dumppacket
  
Sanitizer: thread (TSAN)

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5766392054546432

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: brajkumar@chromium.org
Components: Internals>Services
Labels: -Type-Bug M-65 Test-Predator-Wrong Type-Bug-Regression
Owner: tsepez@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.

Using Code Search for the file, "libc_interceptor.cc" and observed there was some recent changes for the below file.

Suspect CL: https://chromium.googlesource.com/chromium/src/+/b081ec35d6b2296cdb67546404d2565a26283e78

tsepez@ -- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks!

Comment 2 by tsepez@chromium.org, Feb 14 2018

Yes, possibly this CL; there was a small re-ordering of logic in that CL in addition to the renames.  Will investigate.

Comment 3 Deleted

Comment 4 Deleted

Comment 5 Deleted

Comment 6 by tsepez@chromium.org, Feb 14 2018

Duh.  I forgot the underlying syscalls aren't thread safe vs. their _r counterparts.

Comment 7 by tsepez@chromium.org, Feb 14 2018

Components: Blink>WebRTC
Owner: tommi@chromium.org
So this is either external-dependency to replace the localtime with localtime_r in the third_party/usrsctp code, or to ensure this is only called by one thread in webrtc.  

Assigning to chromium-equivalent (I hope) of webrtc OWNERS

Comment 8 by tommi@chromium.org, Jun 18 2018

Cc: tommi@chromium.org
Owner: kwiberg@chromium.org
Karl - this slipped between the cracks, can you take a look? (or reassign?)
Owner: deadbeef@chromium.org
Comment #7 calls for a change in usrsctp (or a change in WebRTC to not call it from more than one thread, but making it thread safe seem both easier and more appealing). Tossing the hot potato to a usrsctp OWNER.
Status: Started (was: Assigned)
Made pull request: https://github.com/sctplab/usrsctp/pull/243

Note that this only happens when verbose logging is enabled; localtime is only called in usrsctp_dumppacket.
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 25 2018

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

commit 462adb280cda73411873a4a0963ce5548759bd9f
Author: Taylor Brandstetter <deadbeef@chromium.org>
Date: Mon Jun 25 18:56:57 2018

Roll src/third_party/usrsctp/usrsctplib/ 159d060dc..7a8bc9a90 (35 commits)

Fixes a buffer overflow bug (see bug link below).

https://chromium.googlesource.com/external/github.com/sctplab/usrsctp/+log/159d060dceec..7a8bc9a90ca9

$ git log 159d060dc..7a8bc9a90 --date=short --no-merges --format='%ad %ae %s'
2018-06-22 deadbeef Switching from localtime to localtime_r on non-Windows platforms.
2018-06-14 tuexen Ensure that the ip6_plen is provided in network byte order.
2018-06-06 tuexen Improve compliance with RFC 4895 and RFC 6458.
2018-06-02 tuexen Don't overflow a buffer if we receive an INIT or INIT-ACK chunk without a RANDOM parameter but with a CHUNKS or HMAC-ALGO parameter. Please note that sending this combination violates the specification.
2018-05-29 tuexen Try to please the Windows compiler.
2018-05-29 tuexen Plug memory leak recently introduced.
2018-05-28 weinrank Suppress 'type-limits' warning for GCC
2018-05-28 tuexen Fix mask.
2018-05-26 tuexen Allow sys tunables to be set before calling usrsctp_init().
2018-05-26 tuexen Add range check for sysctl set functions.
2018-05-21 tuexen Fix typo.
2018-05-21 tuexen Cleanup and sync with FreeBSD sources.
2018-05-21 tuexen Backport https://svnweb.freebsd.org/changeset/base/333813.
2018-05-21 tuexen Whitespace change.
2018-05-14 tuexen Sync FreeBSD ID and minor cleanups.
2018-05-14 tuexen Ensure MTUs are a multiple of 4.
2018-05-14 ruengeler m_last is not needed
2018-05-09 ruengeler Build clusters
2018-05-09 ruengeler Set DF Flag correctly
2018-05-08 tuexen Remove stray =.
2018-05-08 tuexen Don't invalidate the sockets, since even they are closed, the value is used to join the corresponding threads.
2018-05-08 tuexen Fix typos.
2018-05-08 tuexen When reporting ERROR or ABORT chunks, don't use more data that is guaranteed to be contigous. Thanks to Felix Weinrank for finding and reporting this bug by fuzzing the usrsctp stack.
2018-05-08 tuexen sctp_get_mbuf_for_msg() should return NULL if mbuf is too small.
2018-05-06 tuexen Don't dereference a NULL pointer.
2018-05-06 tuexen Handle the case of m == NULL in m_copym().
2018-04-26 justin.kim recv_thread: Invalidate socket after closing
2018-04-08 tuexen Fix logical inversion bug.
2018-04-08 tuexen Small cleanup, no functional change.
2018-04-08 tuexen Fix a signed/unsiged warning.
2018-03-27 deadbeef Fix increment in sctp_connectx_helper_add
2018-03-27 deadbeef Fix cast in sctp_findassociation_ep_addr
2018-03-23 weinrank Fix warnings for "atomic_init"
2018-03-23 weinrank Check for "-Wstrict-prototypes"
2018-03-21 tuexen Use HTTP/SCTP PPID.

Created with:
  roll-dep src/third_party/usrsctp/usrsctplib

Bug:  chromium:854883 , chromium:811477
Change-Id: Ib3db2407c2e591b4162c675596346d8be00955a6
Reviewed-on: https://chromium-review.googlesource.com/1112590
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Taylor Brandstetter <deadbeef@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570114}
[modify] https://crrev.com/462adb280cda73411873a4a0963ce5548759bd9f/DEPS
[modify] https://crrev.com/462adb280cda73411873a4a0963ce5548759bd9f/third_party/usrsctp/README.chromium

Sign in to add a comment