From 058c7db80b074fa52aaa504329ee69cf25a516e3 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Fri, 11 Dec 2020 23:04:32 -0800 Subject: [PATCH] Software: Fix out of bounds accesses in CopyRegion Fixes issue 11393. The problem is that left and top make no sense for a width by height array; they only make sense in a larger array where from which a smaller part is extracted. Thus, the overall size of the array is provided to CopyRegion in addition to the sub-region. EncodeXFB already handles the extraction, so CopyRegion's only use there is to resize the image (and thus no sub-region is provided). --- .../Core/VideoBackends/Software/CopyRegion.h | 46 +++++++++++++------ .../VideoBackends/Software/EfbInterface.cpp | 16 +++---- .../Core/VideoBackends/Software/SWTexture.cpp | 13 ++---- 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/Source/Core/VideoBackends/Software/CopyRegion.h b/Source/Core/VideoBackends/Software/CopyRegion.h index c275dcb38d..2c596befea 100644 --- a/Source/Core/VideoBackends/Software/CopyRegion.h +++ b/Source/Core/VideoBackends/Software/CopyRegion.h @@ -1,33 +1,53 @@ #pragma once +#include "Common/Assert.h" #include "Common/MathUtil.h" #include namespace SW { -// Modified from -// http://tech-algorithm.com/articles/nearest-neighbor-image-scaling/ +// Copies a region of source to a region of destination, performing nearest-neighbor rescaling template -void CopyRegion(const T* const source, const MathUtil::Rectangle& srcrect, T* destination, - const MathUtil::Rectangle& dstrect) +void CopyRegion(const T* const source, const MathUtil::Rectangle& srcrect, const int src_width, + const int src_height, T* destination, const MathUtil::Rectangle& dstrect, + const int dst_width, const int dst_height) { + ASSERT(srcrect.top >= 0 && srcrect.bottom <= src_height); + ASSERT(srcrect.left >= 0 && srcrect.right <= src_width); + ASSERT(dstrect.top >= 0 && dstrect.bottom <= dst_height); + ASSERT(dstrect.left >= 0 && dstrect.right <= dst_width); + + int copy_width = dstrect.GetWidth(); + int copy_height = dstrect.GetHeight(); + double x_ratio = srcrect.GetWidth() / static_cast(dstrect.GetWidth()); double y_ratio = srcrect.GetHeight() / static_cast(dstrect.GetHeight()); - for (int i = 0; i < dstrect.GetHeight(); i++) + + for (int y_off = 0; y_off < copy_height; y_off++) { - for (int j = 0; j < dstrect.GetWidth(); j++) + for (int x_off = 0; x_off < copy_width; x_off++) { - int destination_x = j + dstrect.left; - int destination_y = i + dstrect.top; - int destination_offset = (destination_y * dstrect.GetWidth()) + destination_x; + int dst_x = dstrect.left + x_off; + int dst_y = dstrect.top + y_off; + int dst_offset = (dst_y * dst_width) + dst_x; - double src_x = std::round(destination_x * x_ratio) + srcrect.left; - double src_y = std::round(destination_y * y_ratio) + srcrect.top; - int src_offset = static_cast((src_y * srcrect.GetWidth()) + src_x); + int src_x = srcrect.left + static_cast(std::round(x_off * x_ratio)); + int src_y = srcrect.top + static_cast(std::round(y_off * y_ratio)); + int src_offset = (src_y * src_width) + src_x; - destination[destination_offset] = source[src_offset]; + destination[dst_offset] = source[src_offset]; } } } + +// Copies the entire image from source to destination, performing nearest-neighbor rescaling +template +void CopyRegion(const T* const source, const int src_width, const int src_height, T* destination, + const int dst_width, const int dst_height) +{ + MathUtil::Rectangle srcrect{0, 0, src_width, src_height}; + MathUtil::Rectangle dstrect{0, 0, dst_width, dst_height}; + CopyRegion(source, srcrect, src_width, src_height, destination, dstrect, dst_width, dst_height); +} } // namespace SW diff --git a/Source/Core/VideoBackends/Software/EfbInterface.cpp b/Source/Core/VideoBackends/Software/EfbInterface.cpp index 8ef62d5b03..acf82fc317 100644 --- a/Source/Core/VideoBackends/Software/EfbInterface.cpp +++ b/Source/Core/VideoBackends/Software/EfbInterface.cpp @@ -634,17 +634,13 @@ void EncodeXFB(u8* xfb_in_ram, u32 memory_stride, const MathUtil::Rectangle src_ptr += memory_stride; } - auto dest_rect = - MathUtil::Rectangle{source_rect.left, source_rect.top, source_rect.right, - static_cast(static_cast(source_rect.bottom) * y_scale)}; + const int src_width = source_rect.GetWidth(); + const int src_height = source_rect.GetHeight(); + const int dst_width = src_width; + const int dst_height = src_height * y_scale; - const std::size_t destination_size = dest_rect.GetWidth() * dest_rect.GetHeight() * 2; - static std::vector destination; - destination.resize(dest_rect.GetWidth() * dest_rect.GetHeight()); - - SW::CopyRegion(source.data(), source_rect, destination.data(), dest_rect); - - memcpy(xfb_in_ram, destination.data(), destination_size); + SW::CopyRegion(source.data(), src_width, src_height, reinterpret_cast(xfb_in_ram), + dst_width, dst_height); } bool ZCompare(u16 x, u16 y, u32 z) diff --git a/Source/Core/VideoBackends/Software/SWTexture.cpp b/Source/Core/VideoBackends/Software/SWTexture.cpp index 466da1efd5..eda833c56f 100644 --- a/Source/Core/VideoBackends/Software/SWTexture.cpp +++ b/Source/Core/VideoBackends/Software/SWTexture.cpp @@ -56,15 +56,10 @@ void SWRenderer::ScaleTexture(AbstractFramebuffer* dst_framebuffer, const SWTexture* software_source_texture = static_cast(src_texture); SWTexture* software_dest_texture = static_cast(dst_framebuffer->GetColorAttachment()); - std::vector source_pixels; - source_pixels.resize(src_rect.GetHeight() * src_rect.GetWidth() * 4); - memcpy(source_pixels.data(), software_source_texture->GetData(), source_pixels.size()); - - std::vector destination_pixels; - destination_pixels.resize(dst_rect.GetHeight() * dst_rect.GetWidth() * 4); - - CopyRegion(source_pixels.data(), src_rect, destination_pixels.data(), dst_rect); - memcpy(software_dest_texture->GetData(), destination_pixels.data(), destination_pixels.size()); + CopyRegion(reinterpret_cast(software_source_texture->GetData()), src_rect, + src_texture->GetWidth(), src_texture->GetHeight(), + reinterpret_cast(software_dest_texture->GetData()), dst_rect, + dst_framebuffer->GetWidth(), dst_framebuffer->GetHeight()); } SWTexture::SWTexture(const TextureConfig& tex_config) : AbstractTexture(tex_config)