Skip to content

Commit

Permalink
usb: Fix use-after-free in UsbDeviceHandleWin::Close
Browse files Browse the repository at this point in the history
In UsbDeviceHandleWin::Close() we loop over each outstanding request
and call Abort() to signal failure since the device handle is about to
be closed. In r842380 this loop was changed from checking if requests_
was empty to using iterators. This was incorrect because a range-based
for loop doesn't increment the iterator until after the loop body, at
which point the iterator has been invalidated by the call to erase().

Bug: 1169654
Change-Id: I5631a9dbbf5276d95b29a84a11d2fc7c260090d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2645503
Auto-Submit: Reilly Grant <[email protected]>
Commit-Queue: James Hollyer <[email protected]>
Reviewed-by: James Hollyer <[email protected]>
Cr-Commit-Position: refs/heads/master@{#846845}
  • Loading branch information
reillyeon authored and Chromium LUCI CQ committed Jan 25, 2021
1 parent 685af4c commit 7b2dbd0
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions services/device/usb/usb_device_handle_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,11 @@ void UsbDeviceHandleWin::Close() {
// Aborting requests may run or destroy callbacks holding the last reference
// to this object so hold a reference for the rest of this method.
scoped_refptr<UsbDeviceHandleWin> self(this);
for (const auto& request : requests_)
request->Abort();

// Avoid using an iterator here because Abort() will remove the entry from
// |requests_|.
while (!requests_.empty())
requests_.front()->Abort();

device_ = nullptr;
}
Expand Down

0 comments on commit 7b2dbd0

Please sign in to comment.