From ea3e3215e0d0e6c986788ec931a0499ed05930b8 Mon Sep 17 00:00:00 2001 From: Joe Downing Date: Fri, 16 Oct 2020 09:15:21 -0700 Subject: [PATCH] Fixing ASAN container-overflow error in DxgiOutputDuplicator The DxgiOutputDuplicator uses a vector to hold the rects that have changed on the screen. It then iterates over the vector to grab each rect and apply it to the updated_region. There is vector resizing logic which checks the 'capacity' of the vector and determines whether there is enough space for the changed rect data. Often the 'capacity' and 'size' of the vector are equal but that isn't always true. When the capacity is greater than size, and the number of changed rects is high enough, rect data will be written past the element pointed to by (data() + size()) and this is the error that ASAN is warning of. The fix is to use size() instead of capacity() when determining whether to resize the vector and as the buffer size we provide to the Windows API. There are no other usages of this vector so there aren't any problems caused by the size/capacity discrepancy in the existing builds. The ASAN issue is worth fixing in case someone comes along and decides to use the vector differently (e.g rely on the size instead of capacity so some of the rects are not counted). Bug: chromium:1138446 Change-Id: I3041091423de889e0f8aabc56ece9466a3000b4f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188900 Reviewed-by: Jamie Walch Commit-Queue: Joe Downing Cr-Commit-Position: refs/heads/master@{#32425} --- modules/desktop_capture/win/dxgi_output_duplicator.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/desktop_capture/win/dxgi_output_duplicator.cc b/modules/desktop_capture/win/dxgi_output_duplicator.cc index c90e2f1ab5..65a0d77667 100644 --- a/modules/desktop_capture/win/dxgi_output_duplicator.cc +++ b/modules/desktop_capture/win/dxgi_output_duplicator.cc @@ -271,7 +271,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion( return false; } - if (metadata_.capacity() < frame_info.TotalMetadataBufferSize) { + if (metadata_.size() < frame_info.TotalMetadataBufferSize) { metadata_.clear(); // Avoid data copy metadata_.resize(frame_info.TotalMetadataBufferSize); } @@ -281,7 +281,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion( reinterpret_cast(metadata_.data()); size_t move_rects_count = 0; _com_error error = duplication_->GetFrameMoveRects( - static_cast(metadata_.capacity()), move_rects, &buff_size); + static_cast(metadata_.size()), move_rects, &buff_size); if (error.Error() != S_OK) { RTC_LOG(LS_ERROR) << "Failed to get move rectangles: " << desktop_capture::utils::ComErrorToString(error); @@ -292,8 +292,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion( RECT* dirty_rects = reinterpret_cast(metadata_.data() + buff_size); size_t dirty_rects_count = 0; error = duplication_->GetFrameDirtyRects( - static_cast(metadata_.capacity()) - buff_size, dirty_rects, - &buff_size); + static_cast(metadata_.size()) - buff_size, dirty_rects, &buff_size); if (error.Error() != S_OK) { RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles: " << desktop_capture::utils::ComErrorToString(error);