Posible access to unintended variable in "mojo/edk/system/core.cc" line 971
Reported by
pet...@gmail.com,
Oct 19 2017
|
||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:56.0) Gecko/20100101 Firefox/56.0
Steps to reproduce the problem:
While experimenting with a CodeSonar plugin we develop, we noticed a
potential bug in file "mojo/edk/system/core.cc" line 971 method Core::DuplicateBufferHandle
*new_buffer_handle = AddDispatcher(new_dispatcher);
if (*new_buffer_handle == MOJO_HANDLE_INVALID) {
LOG(ERROR) << "Handle table full";
dispatcher->Close(); //HERE
return MOJO_RESULT_RESOURCE_EXHAUSTED;
}
Shouldn't new_dispatcher be closed (instead of dispatcher) since it is the one that cannot be added to the table?
Thanks,
Petru Florin Mihancea
What is the expected behavior?
What went wrong?
The problem has been detected automatically via static analysis.
Did this work before? N/A
Chrome version: Master branch Channel: n/a
OS Version: OS X 10.12
Flash Version: Shockwave Flash 27.0 r0
,
Oct 20 2017
,
Oct 23 2017
Thanks, good catch! This is code definitely incorrect as-is.
,
Oct 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/027bc13a7481ee005d535e7fcd433990a0924343 commit 027bc13a7481ee005d535e7fcd433990a0924343 Author: Ken Rockot <rockot@chromium.org> Date: Mon Oct 23 19:45:44 2017 Fix MojoDuplicateBufferHandle failure behavior If handle duplication fails due to handle exhaustion we would incorrectly close the source handle's dispatcher rather than the newly allocated dispatcher. That's no good, and this does the right thing instead. In practice it's unlikely that an instance of Chrome has ever actually run out of Mojo handles (and many other things would break as a result in that case anyway), so this is merely fixing a hypothetical bug. Bug: 776293 Change-Id: I4872d10b7261721d6f99836ec95c80d4ed385d77 Reviewed-on: https://chromium-review.googlesource.com/733851 Reviewed-by: Jay Civelli <jcivelli@chromium.org> Commit-Queue: Ken Rockot <rockot@chromium.org> Cr-Commit-Position: refs/heads/master@{#510876} [modify] https://crrev.com/027bc13a7481ee005d535e7fcd433990a0924343/mojo/edk/system/core.cc
,
Oct 23 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by rsesek@chromium.org
, Oct 20 2017Labels: -OS-Mac