From 0ed17bbe25f5c24aab900926b26e8b8946e7cee5 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 20 Mar 2024 23:00:10 +1000 Subject: [PATCH 1/8] Limit bmp sizes to match browser max. --- src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs | 10 ++++++++++ tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs | 9 +++++++++ tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Bmp/issue-2696.bmp | 3 +++ 4 files changed, 24 insertions(+) create mode 100644 tests/Images/Input/Bmp/issue-2696.bmp diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index 863fed359c..b99d8cef88 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -1422,6 +1422,16 @@ private int ReadImageHeaders(BufferedReadStream stream, out bool inverted, out b this.ReadFileHeader(stream); this.ReadInfoHeader(stream); + // BMPs with negative-or-zero width are invalid. Also, reject extremely wide images + // to keep the math sane. And reject int.MinValue as a height because you can't + // get its absolute value (because -int.MinValue is one more than int.MaxValue). + const int k64KWidth = 65535; + bool sizeOk = this.infoHeader.Width > 0 && this.infoHeader.Width <= k64KWidth && this.infoHeader.Height != int.MinValue; + if (!sizeOk) + { + BmpThrowHelper.ThrowInvalidImageContentException($"Invalid file header dimensions found. {this.infoHeader.Width}x{this.infoHeader.Height}px."); + } + // see http://www.drdobbs.com/architecture-and-design/the-bmp-file-format-part-1/184409517 // If the height is negative, then this is a Windows bitmap whose origin // is the upper-left corner and not the lower-left. The inverted flag diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index e76f21d0e9..7e89bc1ea6 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -558,4 +558,13 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder.Instance); + }); } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fe633fddc3..c1da2ce540 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -441,6 +441,8 @@ public static class Bmp public const string BlackWhitePalletDataMatrix = "Bmp/bit1datamatrix.bmp"; + public const string Issue2696 = "Bmp/issue-2696.bmp"; + public static readonly string[] BitFields = { Rgb32bfdef, diff --git a/tests/Images/Input/Bmp/issue-2696.bmp b/tests/Images/Input/Bmp/issue-2696.bmp new file mode 100644 index 0000000000..6770dd9469 --- /dev/null +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bc42cda9bac8fc73351ad03bf55214069bb8d31ea5bdd806321a8cc8b56c282e +size 126 From 4e164a59657645d0bc449631c1e05f7cb8dc5523 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 21 Mar 2024 21:45:22 +1000 Subject: [PATCH 2/8] Use MemoryAllocator directly to limit all allocations. --- src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs | 10 -------- .../ComponentProcessors/ComponentProcessor.cs | 2 +- .../Components/Encoder/ComponentProcessor.cs | 2 +- .../Memory/Allocators/MemoryAllocator.cs | 15 +++++++++++- .../Allocators/SimpleGcMemoryAllocator.cs | 6 ++--- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 13 ++++------ .../Allocators/UnmanagedMemoryAllocator.cs | 4 +++- .../Memory/MemoryAllocatorExtensions.cs | 22 +++++++++++------ .../Transforms/Resize/ResizeKernelMap.cs | 2 +- .../Transforms/Resize/ResizeWorker.cs | 2 +- .../Formats/Bmp/BmpDecoderTests.cs | 2 +- .../SimpleGcMemoryAllocatorTests.cs | 1 + ...niformUnmanagedPoolMemoryAllocatorTests.cs | 9 +++++++ .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 24 ++++++++++++++++--- 14 files changed, 76 insertions(+), 38 deletions(-) diff --git a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs index b99d8cef88..863fed359c 100644 --- a/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs +++ b/src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs @@ -1422,16 +1422,6 @@ private int ReadImageHeaders(BufferedReadStream stream, out bool inverted, out b this.ReadFileHeader(stream); this.ReadInfoHeader(stream); - // BMPs with negative-or-zero width are invalid. Also, reject extremely wide images - // to keep the math sane. And reject int.MinValue as a height because you can't - // get its absolute value (because -int.MinValue is one more than int.MaxValue). - const int k64KWidth = 65535; - bool sizeOk = this.infoHeader.Width > 0 && this.infoHeader.Width <= k64KWidth && this.infoHeader.Height != int.MinValue; - if (!sizeOk) - { - BmpThrowHelper.ThrowInvalidImageContentException($"Invalid file header dimensions found. {this.infoHeader.Width}x{this.infoHeader.Height}px."); - } - // see http://www.drdobbs.com/architecture-and-design/the-bmp-file-format-part-1/184409517 // If the height is negative, then this is a Windows bitmap whose origin // is the upper-left corner and not the lower-left. The inverted flag diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs index f0cc4d5e82..3bdaf2dc27 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/ComponentProcessors/ComponentProcessor.cs @@ -16,7 +16,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, JpegFrame frame, Size this.Component = component; this.BlockAreaSize = component.SubSamplingDivisors * blockSize; - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, this.BlockAreaSize.Height); diff --git a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs index c33a8a1968..9346b11e7b 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Encoder/ComponentProcessor.cs @@ -28,7 +28,7 @@ public ComponentProcessor(MemoryAllocator memoryAllocator, Component component, this.blockAreaSize = component.SubSamplingDivisors * 8; // alignment of 8 so each block stride can be sampled from a single 'ref pointer' - this.ColorBuffer = memoryAllocator.Allocate2DOveraligned( + this.ColorBuffer = memoryAllocator.Allocate2DOverAligned( postProcessorBufferSize.Width, postProcessorBufferSize.Height, 8, diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 2bd9cb5eef..3d89fc9882 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -20,6 +20,18 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); + /// + /// Gets or sets the maximum allowable allocatable size of a 2 dimensional buffer. + /// Defaults to 65535 * 65535. + /// + public Size MaxAllocatableSize2D { get; set; } = new Size(ushort.MaxValue, ushort.MaxValue); + + /// + /// Gets or sets the maximum allowable allocatable size of a 1 dimensional buffer. + /// + /// Defaults to 65535 * 4. + public int MaxAllocatableSize1D { get; set; } = ushort.MaxValue * 4; + /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. /// @@ -42,7 +54,7 @@ public static MemoryAllocator Create(MemoryAllocatorOptions options) => new UniformUnmanagedMemoryPoolMemoryAllocator(options.MaximumPoolSizeMegabytes); /// - /// Allocates an , holding a of length . + /// Allocates an , holding a of length . /// /// Type of the data stored in the buffer. /// Size of the buffer to allocate. @@ -64,6 +76,7 @@ public virtual void ReleaseRetainedResources() /// /// Allocates a . /// + /// Type of the data stored in the buffer. /// The total length of the buffer. /// The expected alignment (eg. to make sure image rows fit into single buffers). /// The . diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 41730d9678..0cc1cc210c 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Six Labors Split License. using System.Buffers; @@ -7,7 +7,7 @@ namespace SixLabors.ImageSharp.Memory; /// -/// Implements by newing up managed arrays on every allocation request. +/// Implements by creating new managed arrays on every allocation request. /// public sealed class SimpleGcMemoryAllocator : MemoryAllocator { @@ -17,7 +17,7 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + Guard.MustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); return new BasicArrayBuffer(new T[length]); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 0bae193632..967eaa3a50 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -79,16 +79,14 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; /// - public override IMemoryOwner Allocate( - int length, - AllocationOptions options = AllocationOptions.None) + public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + Guard.MustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer(length); + SharedArrayPoolBuffer buffer = new(length); if (options.Has(AllocationOptions.Clean)) { buffer.GetSpan().Clear(); @@ -102,8 +100,7 @@ public override IMemoryOwner Allocate( UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) { - UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, length, options.Has(AllocationOptions.Clean)); - return buffer; + return this.pool.CreateGuardedBuffer(mem, length, options.Has(AllocationOptions.Clean)); } } @@ -124,7 +121,7 @@ internal override MemoryGroup AllocateGroup( if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer((int)totalLength); + SharedArrayPoolBuffer buffer = new((int)totalLength); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } diff --git a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs index da202aa596..1b9c2779f3 100644 --- a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs @@ -20,7 +20,9 @@ internal class UnmanagedMemoryAllocator : MemoryAllocator public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - var buffer = UnmanagedBuffer.Allocate(length); + Guard.MustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + + UnmanagedBuffer buffer = UnmanagedBuffer.Allocate(length); if (options.Has(AllocationOptions.Clean)) { buffer.GetSpan().Clear(); diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index ff306e1e45..93693f5edf 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -18,20 +18,23 @@ public static class MemoryAllocatorExtensions /// The memory allocator. /// The buffer width. /// The buffer height. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, int width, int height, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct { + Guard.MustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); + Guard.MustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + long groupLength = (long)width * height; MemoryGroup memoryGroup; - if (preferContiguosImageBuffers && groupLength < int.MaxValue) + if (preferContiguousImageBuffers && groupLength < int.MaxValue) { IMemoryOwner buffer = memoryAllocator.Allocate((int)groupLength, options); memoryGroup = MemoryGroup.CreateContiguous(buffer, false); @@ -69,16 +72,16 @@ public static Buffer2D Allocate2D( /// The type of buffer items to allocate. /// The memory allocator. /// The buffer size. - /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . + /// A value indicating whether the allocated buffer should be contiguous, unless bigger than . /// The allocation options. /// The . public static Buffer2D Allocate2D( this MemoryAllocator memoryAllocator, Size size, - bool preferContiguosImageBuffers, + bool preferContiguousImageBuffers, AllocationOptions options = AllocationOptions.None) where T : struct => - Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguosImageBuffers, options); + Allocate2D(memoryAllocator, size.Width, size.Height, preferContiguousImageBuffers, options); /// /// Allocates a buffer of value type objects interpreted as a 2D region @@ -96,7 +99,7 @@ public static Buffer2D Allocate2D( where T : struct => Allocate2D(memoryAllocator, size.Width, size.Height, false, options); - internal static Buffer2D Allocate2DOveraligned( + internal static Buffer2D Allocate2DOverAligned( this MemoryAllocator memoryAllocator, int width, int height, @@ -104,6 +107,9 @@ internal static Buffer2D Allocate2DOveraligned( AllocationOptions options = AllocationOptions.None) where T : struct { + Guard.MustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); + Guard.MustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + long groupLength = (long)width * height; MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( groupLength, @@ -127,6 +133,8 @@ internal static IMemoryOwner AllocatePaddedPixelRowBuffer( int paddingInBytes) { int length = (width * pixelSizeInBytes) + paddingInBytes; + Guard.MustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1D, nameof(length)); + return memoryAllocator.Allocate(length); } } diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs index c1907bb520..da26cbefcd 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeKernelMap.cs @@ -50,7 +50,7 @@ private ResizeKernelMap( this.sourceLength = sourceLength; this.DestinationLength = destinationLength; this.MaxDiameter = (radius * 2) + 1; - this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguosImageBuffers: true, AllocationOptions.Clean); + this.data = memoryAllocator.Allocate2D(this.MaxDiameter, bufferHeight, preferContiguousImageBuffers: true, AllocationOptions.Clean); this.pinHandle = this.data.DangerousGetSingleMemory().Pin(); this.kernels = new ResizeKernel[destinationLength]; this.tempValues = new double[this.MaxDiameter]; diff --git a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs index cce27a401c..fd13d7b5a6 100644 --- a/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs +++ b/src/ImageSharp/Processing/Processors/Transforms/Resize/ResizeWorker.cs @@ -83,7 +83,7 @@ public ResizeWorker( this.transposedFirstPassBuffer = configuration.MemoryAllocator.Allocate2D( this.workerHeight, targetWorkingRect.Width, - preferContiguosImageBuffers: true, + preferContiguousImageBuffers: true, options: AllocationOptions.Clean); this.tempRowBuffer = configuration.MemoryAllocator.Allocate(this.sourceRectangle.Width); diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index 7e89bc1ea6..8d72581f68 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -563,7 +563,7 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel - => Assert.Throws(() => + => Assert.Throws(() => { using Image image = provider.GetImage(BmpDecoder.Instance); }); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 780ba7f20e..01ee211edc 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -21,6 +21,7 @@ public BufferTests() [Theory] [InlineData(-1)] + [InlineData((ushort.MaxValue * 4) + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) { ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 7e7c136635..709acefc34 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -114,6 +114,15 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); } + [Theory] + [InlineData(-1)] + [InlineData((ushort.MaxValue * 4) + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + { + ArgumentOutOfRangeException ex = Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); + Assert.Equal("length", ex.ParamName); + } + [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() { diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 5364de0652..1a865e46cb 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -72,11 +72,11 @@ public void Construct_PreferContiguousImageBuffers_AllocatesContiguousRegardless using Buffer2D buffer = useSizeOverload ? this.MemoryAllocator.Allocate2D( new Size(200, 200), - preferContiguosImageBuffers: true) : + preferContiguousImageBuffers: true) : this.MemoryAllocator.Allocate2D( 200, 200, - preferContiguosImageBuffers: true); + preferContiguousImageBuffers: true); Assert.Equal(1, buffer.FastMemoryGroup.Count); Assert.Equal(200 * 200, buffer.FastMemoryGroup.TotalLength); } @@ -87,7 +87,7 @@ public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int { this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOveraligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -337,4 +337,22 @@ public void PublicMemoryGroup_IsMemoryGroupView() Assert.False(mgBefore.IsValid); Assert.NotSame(mgBefore, buffer1.MemoryGroup); } + + [Theory] + [InlineData(-1)] + [InlineData(ushort.MaxValue + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(length, length)); + + [Theory] + [InlineData(-1)] + [InlineData(ushort.MaxValue + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_Size(int length) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(length, length))); + + [Theory] + [InlineData(-1)] + [InlineData(ushort.MaxValue + 1)] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_OverAligned(int length) + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(length, length, 1)); } From 9cdcc4f8e9980972746760db7e3b8d434fd13485 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 21 Mar 2024 22:01:15 +1000 Subject: [PATCH 3/8] Bump 1D limit --- src/ImageSharp/Memory/Allocators/MemoryAllocator.cs | 4 ++-- .../Memory/Allocators/SimpleGcMemoryAllocatorTests.cs | 2 +- .../Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 3d89fc9882..465cd7a61d 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -29,8 +29,8 @@ public abstract class MemoryAllocator /// /// Gets or sets the maximum allowable allocatable size of a 1 dimensional buffer. /// - /// Defaults to 65535 * 4. - public int MaxAllocatableSize1D { get; set; } = ushort.MaxValue * 4; + /// Defaults to 65535 * 64. + public int MaxAllocatableSize1D { get; set; } = ushort.MaxValue * 64; /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 01ee211edc..0884100f09 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -21,7 +21,7 @@ public BufferTests() [Theory] [InlineData(-1)] - [InlineData((ushort.MaxValue * 4) + 1)] + [InlineData((ushort.MaxValue * 64) + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) { ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 709acefc34..06aaae6761 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -116,7 +116,7 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati [Theory] [InlineData(-1)] - [InlineData((ushort.MaxValue * 4) + 1)] + [InlineData((ushort.MaxValue * 64) + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) { ArgumentOutOfRangeException ex = Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); From 8806189f20965de701bc999c22f3e1ba028b47fd Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Thu, 21 Mar 2024 22:36:26 +1000 Subject: [PATCH 4/8] Fix all tests --- .../Memory/Allocators/MemoryAllocator.cs | 14 ++++++++++++-- .../Memory/Allocators/SimpleGcMemoryAllocator.cs | 2 +- .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 2 +- .../Memory/Allocators/UnmanagedMemoryAllocator.cs | 2 +- src/ImageSharp/Memory/MemoryAllocatorExtensions.cs | 10 +++++----- .../Formats/Bmp/BmpDecoderTests.cs | 2 +- .../Formats/Bmp/BmpEncoderTests.cs | 9 ++++++--- .../Allocators/SimpleGcMemoryAllocatorTests.cs | 7 ++----- .../UniformUnmanagedPoolMemoryAllocatorTests.cs | 7 ++----- tests/ImageSharp.Tests/Memory/Buffer2DTests.cs | 6 +++--- 10 files changed, 34 insertions(+), 27 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 465cd7a61d..341dacb38d 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -29,8 +29,8 @@ public abstract class MemoryAllocator /// /// Gets or sets the maximum allowable allocatable size of a 1 dimensional buffer. /// - /// Defaults to 65535 * 64. - public int MaxAllocatableSize1D { get; set; } = ushort.MaxValue * 64; + /// Defaults to 1073741823. + public int MaxAllocatableSize1D { get; set; } = int.MaxValue / 2; /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. @@ -88,4 +88,14 @@ internal virtual MemoryGroup AllocateGroup( AllocationOptions options = AllocationOptions.None) where T : struct => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); + + internal static void MemoryGuardMustBeBetweenOrEqualTo(int value, int min, int max, string paramName) + { + if (value >= min && value <= max) + { + return; + } + + throw new InvalidMemoryOperationException($"Parameter \"{paramName}\" must be between or equal to {min} and {max}, was {value}"); + } } diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 0cc1cc210c..f979ebf000 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -17,7 +17,7 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); return new BasicArrayBuffer(new T[length]); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 967eaa3a50..e410a0dbca 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -81,7 +81,7 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) diff --git a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs index 1b9c2779f3..62e14affba 100644 --- a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs @@ -20,7 +20,7 @@ internal class UnmanagedMemoryAllocator : MemoryAllocator public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); UnmanagedBuffer buffer = UnmanagedBuffer.Allocate(length); if (options.Has(AllocationOptions.Clean)) diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index 93693f5edf..42359d029f 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -29,8 +29,8 @@ public static Buffer2D Allocate2D( AllocationOptions options = AllocationOptions.None) where T : struct { - Guard.MustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); - Guard.MustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); long groupLength = (long)width * height; MemoryGroup memoryGroup; @@ -107,8 +107,8 @@ internal static Buffer2D Allocate2DOverAligned( AllocationOptions options = AllocationOptions.None) where T : struct { - Guard.MustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); - Guard.MustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); long groupLength = (long)width * height; MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( @@ -133,7 +133,7 @@ internal static IMemoryOwner AllocatePaddedPixelRowBuffer( int paddingInBytes) { int length = (width * pixelSizeInBytes) + paddingInBytes; - Guard.MustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1D, nameof(length)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1D, nameof(length)); return memoryAllocator.Allocate(length); } diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index 8d72581f68..7e89bc1ea6 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -563,7 +563,7 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) where TPixel : unmanaged, IPixel - => Assert.Throws(() => + => Assert.Throws(() => { using Image image = provider.GetImage(BmpDecoder.Instance); }); diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs index 42cbd90f3b..3d6aab9bcd 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs @@ -3,6 +3,7 @@ using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Bmp; +using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Metadata; using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.Processing; @@ -349,13 +350,15 @@ public void Encode_PreservesColorProfile(TestImageProvider provi } [Theory] - [InlineData(1, 66535)] - [InlineData(66535, 1)] + [InlineData(1, 65536)] + [InlineData(65536, 1)] public void Encode_WorksWithSizeGreaterThen65k(int width, int height) { Exception exception = Record.Exception(() => { - using Image image = new Image(width, height); + Configuration c = Configuration.CreateDefaultInstance(); + c.MemoryAllocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null) { MaxAllocatableSize2D = new(65537, 65537) }; + using Image image = new Image(c, width, height); using MemoryStream memStream = new(); image.Save(memStream, BmpEncoder); }); diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 0884100f09..b00dfa392a 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -21,12 +21,9 @@ public BufferTests() [Theory] [InlineData(-1)] - [InlineData((ushort.MaxValue * 64) + 1)] + [InlineData(1073741823 + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) - { - ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); - Assert.Equal("length", ex.ParamName); - } + => Assert.Throws(() => this.MemoryAllocator.Allocate(length)); [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 06aaae6761..d6ccfa8399 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -116,12 +116,9 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati [Theory] [InlineData(-1)] - [InlineData((ushort.MaxValue * 64) + 1)] + [InlineData(1073741823 + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) - { - ArgumentOutOfRangeException ex = Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); - Assert.Equal("length", ex.ParamName); - } + => Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index 1a865e46cb..a73a3c23ec 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -342,17 +342,17 @@ public void PublicMemoryGroup_IsMemoryGroupView() [InlineData(-1)] [InlineData(ushort.MaxValue + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate2D(length, length)); + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(length, length)); [Theory] [InlineData(-1)] [InlineData(ushort.MaxValue + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_Size(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(length, length))); + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(length, length))); [Theory] [InlineData(-1)] [InlineData(ushort.MaxValue + 1)] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_OverAligned(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(length, length, 1)); + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(length, length, 1)); } From ebfe761a26ff22cc757355e2a0ad75fdf71e5d16 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 23 Mar 2024 22:00:16 +1000 Subject: [PATCH 5/8] Use smarter per-environment limits --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 + .../Memory/Allocators/MemoryAllocator.cs | 55 +++++++++++-- .../Allocators/SimpleGcMemoryAllocator.cs | 2 +- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 2 +- .../Allocators/UnmanagedMemoryAllocator.cs | 2 +- .../Memory/MemoryAllocatorExtensions.cs | 10 +-- .../Formats/Bmp/BmpEncoderTests.cs | 17 ++-- .../Formats/Jpg/JpegDecoderTests.cs | 15 +--- .../SimpleGcMemoryAllocatorTests.cs | 15 ++-- ...niformUnmanagedPoolMemoryAllocatorTests.cs | 9 ++- .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 77 ++++++++++--------- 11 files changed, 129 insertions(+), 78 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a96c53c104..abd5cadeb1 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1968,6 +1968,9 @@ private IMemoryOwner ReadChunkData(int length) } // We rent the buffer here to return it afterwards in Decode() + // We don't want to throw a degenerated memory exception here as we want to allow partial decoding + // so limit the length. + length = (int)Math.Min(length, this.currentStream.Length - this.currentStream.Position); IMemoryOwner buffer = this.configuration.MemoryAllocator.Allocate(length, AllocationOptions.Clean); this.currentStream.Read(buffer.GetSpan(), 0, length); diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 341dacb38d..ac1f3866b7 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -2,6 +2,8 @@ // Licensed under the Six Labors Split License. using System.Buffers; +using System.Runtime.CompilerServices; +using SixLabors.ImageSharp.PixelFormats; namespace SixLabors.ImageSharp.Memory; @@ -10,6 +12,16 @@ namespace SixLabors.ImageSharp.Memory; /// public abstract class MemoryAllocator { + /// + /// Gets the default max allocatable size of a 1D buffer in bytes. + /// + public static readonly int DefaultMaxAllocatableSize1DInBytes = GetDefaultMaxAllocatableSize1DInBytes(); + + /// + /// Gets the default max allocatable size of a 2D buffer in bytes. + /// + public static readonly Size DefaultMaxAllocatableSize2DInBytes = GetDefaultMaxAllocatableSize2DInBytes(); + /// /// Gets the default platform-specific global instance that /// serves as the default value for . @@ -22,15 +34,15 @@ public abstract class MemoryAllocator /// /// Gets or sets the maximum allowable allocatable size of a 2 dimensional buffer. - /// Defaults to 65535 * 65535. + /// Defaults to . /// - public Size MaxAllocatableSize2D { get; set; } = new Size(ushort.MaxValue, ushort.MaxValue); + public Size MaxAllocatableSize2DInBytes { get; set; } = DefaultMaxAllocatableSize2DInBytes; /// /// Gets or sets the maximum allowable allocatable size of a 1 dimensional buffer. /// - /// Defaults to 1073741823. - public int MaxAllocatableSize1D { get; set; } = int.MaxValue / 2; + /// Defaults to . + public int MaxAllocatableSize1DInBytes { get; set; } = DefaultMaxAllocatableSize1DInBytes; /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. @@ -89,13 +101,42 @@ internal virtual MemoryGroup AllocateGroup( where T : struct => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); - internal static void MemoryGuardMustBeBetweenOrEqualTo(int value, int min, int max, string paramName) + internal static void MemoryGuardMustBeBetweenOrEqualTo(int value, int min, int max, string paramName) + where T : struct { - if (value >= min && value <= max) + int typeSizeInBytes = Unsafe.SizeOf(); + long valueInBytes = value * typeSizeInBytes; + + // If a sufficiently large value is passed in, the multiplication will overflow. + // We can detect this by checking if the result is less than the original value. + if (valueInBytes < value && value > 0) + { + valueInBytes = long.MaxValue; + } + + if (valueInBytes >= min && valueInBytes <= max) { return; } - throw new InvalidMemoryOperationException($"Parameter \"{paramName}\" must be between or equal to {min} and {max}, was {value}"); + throw new InvalidMemoryOperationException($"Parameter \"{paramName}\" must be between or equal to {min} and {max}, was {valueInBytes}"); + } + + private static Size GetDefaultMaxAllocatableSize2DInBytes() + { + // Limit dimensions to 65535x65535 and 32767x32767 @ 4 bytes per pixel for 64 and 32 bit processes respectively. + int maxLength = Environment.Is64BitProcess ? ushort.MaxValue : short.MaxValue; + int maxLengthInRgba32Bytes = maxLength * Unsafe.SizeOf(); + return new(maxLengthInRgba32Bytes, maxLengthInRgba32Bytes); + } + + private static int GetDefaultMaxAllocatableSize1DInBytes() + { + // It's possible to require buffers that are not related to image dimensions. + // For example, when we need to allocate buffers for IDAT chunks in PNG files or when allocating + // cache buffers for image quantization. + // Limit the maximum buffer size to 64MB for 64bit processes and 32MB for 32 bit processes. + int limitInMB = Environment.Is64BitProcess ? 64 : 32; + return limitInMB * 1024 * 1024; } } diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index f979ebf000..85d2ce9c58 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -17,7 +17,7 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1DInBytes, nameof(length)); return new BasicArrayBuffer(new T[length]); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index e410a0dbca..fefe8b6688 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -81,7 +81,7 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1DInBytes, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) diff --git a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs index 62e14affba..1484cc7329 100644 --- a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs @@ -20,7 +20,7 @@ internal class UnmanagedMemoryAllocator : MemoryAllocator public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1D, nameof(length)); + MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1DInBytes, nameof(length)); UnmanagedBuffer buffer = UnmanagedBuffer.Allocate(length); if (options.Has(AllocationOptions.Clean)) diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index 42359d029f..af84945abf 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -29,8 +29,8 @@ public static Buffer2D Allocate2D( AllocationOptions options = AllocationOptions.None) where T : struct { - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Width, nameof(width)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Height, nameof(height)); long groupLength = (long)width * height; MemoryGroup memoryGroup; @@ -107,8 +107,8 @@ internal static Buffer2D Allocate2DOverAligned( AllocationOptions options = AllocationOptions.None) where T : struct { - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2D.Width, nameof(width)); - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2D.Height, nameof(height)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Width, nameof(width)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Height, nameof(height)); long groupLength = (long)width * height; MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( @@ -133,7 +133,7 @@ internal static IMemoryOwner AllocatePaddedPixelRowBuffer( int paddingInBytes) { int length = (width * pixelSizeInBytes) + paddingInBytes; - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1D, nameof(length)); + MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1DInBytes, nameof(length)); return memoryAllocator.Allocate(length); } diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs index 3d6aab9bcd..12bc547a75 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs @@ -349,16 +349,21 @@ public void Encode_PreservesColorProfile(TestImageProvider provi Assert.Equal(expectedProfileBytes, actualProfileBytes); } + public static TheoryData Encode_WorksWithSizeGreaterThen65k_Data { get; set; } = new() + { + { new Size(1, MemoryAllocator.DefaultMaxAllocatableSize2DInBytes.Height + 1) }, + { new Size(MemoryAllocator.DefaultMaxAllocatableSize2DInBytes.Width + 1, 1) } + }; + [Theory] - [InlineData(1, 65536)] - [InlineData(65536, 1)] - public void Encode_WorksWithSizeGreaterThen65k(int width, int height) + [MemberData(nameof(Encode_WorksWithSizeGreaterThen65k_Data))] + public void Encode_WorksWithSizeGreaterThen65k(Size size) { Exception exception = Record.Exception(() => { - Configuration c = Configuration.CreateDefaultInstance(); - c.MemoryAllocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null) { MaxAllocatableSize2D = new(65537, 65537) }; - using Image image = new Image(c, width, height); + Configuration configuration = Configuration.CreateDefaultInstance(); + configuration.MemoryAllocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null) { MaxAllocatableSize2DInBytes = size + new Size(1, 1) }; + using Image image = new Image(configuration, size.Width, size.Height); using MemoryStream memStream = new(); image.Save(memStream, BmpEncoder); }); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 6a94a98ac6..972cabe242 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -339,20 +339,9 @@ public void Issue2564_DecodeWorks(TestImageProvider provider) [Theory] [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)] - public void DecodeHang(TestImageProvider provider) + public void DecodeHang_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel - { - if (TestEnvironment.IsWindows && - TestEnvironment.RunsOnCI) - { - // Windows CI runs consistently fail with OOM. - return; - } - - using Image image = provider.GetImage(JpegDecoder.Instance); - Assert.Equal(65503, image.Width); - Assert.Equal(65503, image.Height); - } + => Assert.Throws(() => { using Image image = provider.GetImage(JpegDecoder.Instance); }); // https://github.com/SixLabors/ImageSharp/issues/2517 [Theory] diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index b00dfa392a..405ed13754 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -17,18 +17,23 @@ public BufferTests() } } - protected SimpleGcMemoryAllocator MemoryAllocator { get; } = new SimpleGcMemoryAllocator(); + protected SimpleGcMemoryAllocator TestMemoryAllocator { get; } = new SimpleGcMemoryAllocator(); + + public static TheoryData InvalidLengths { get; set; } = new() + { + { -1 }, + { MemoryAllocator.DefaultMaxAllocatableSize1DInBytes + 1 } + }; [Theory] - [InlineData(-1)] - [InlineData(1073741823 + 1)] + [MemberData(nameof(InvalidLengths))] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate(length)); + => Assert.Throws(() => this.TestMemoryAllocator.Allocate(length)); [Fact] public unsafe void Allocate_MemoryIsPinnableMultipleTimes() { - SimpleGcMemoryAllocator allocator = this.MemoryAllocator; + SimpleGcMemoryAllocator allocator = this.TestMemoryAllocator; using IMemoryOwner memoryOwner = allocator.Allocate(100); using (MemoryHandle pin = memoryOwner.Memory.Pin()) diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index d6ccfa8399..f8006a35d7 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -114,9 +114,14 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); } + public static TheoryData InvalidLengths { get; set; } = new() + { + { -1 }, + { MemoryAllocator.DefaultMaxAllocatableSize1DInBytes + 1 } + }; + [Theory] - [InlineData(-1)] - [InlineData(1073741823 + 1)] + [MemberData(nameof(InvalidLengths))] public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) => Assert.Throws(() => new UniformUnmanagedMemoryPoolMemoryAllocator(null).Allocate(length)); diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index a73a3c23ec..cecd79c2e6 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -23,7 +23,7 @@ public static void SpanPointsTo(Span span, Memory buffer, int bufferOff } } - private TestMemoryAllocator MemoryAllocator { get; } = new TestMemoryAllocator(); + private TestMemoryAllocator TestMemoryAllocator { get; } = new TestMemoryAllocator(); private const int Big = 99999; @@ -33,9 +33,9 @@ public static void SpanPointsTo(Span span, Memory buffer, int bufferOff [InlineData(300, 42, 777)] public unsafe void Construct(int bufferCapacity, int width, int height) { - this.MemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; + this.TestMemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; - using (Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height)) + using (Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(width, height)) { Assert.Equal(width, buffer.Width); Assert.Equal(height, buffer.Height); @@ -51,9 +51,9 @@ public unsafe void Construct(int bufferCapacity, int width, int height) [InlineData(3, 0, 0)] public unsafe void Construct_Empty(int bufferCapacity, int width, int height) { - this.MemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; + this.TestMemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; - using (Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height)) + using (Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(width, height)) { Assert.Equal(width, buffer.Width); Assert.Equal(height, buffer.Height); @@ -67,13 +67,13 @@ public unsafe void Construct_Empty(int bufferCapacity, int width, int height) [InlineData(true)] public void Construct_PreferContiguousImageBuffers_AllocatesContiguousRegardlessOfCapacity(bool useSizeOverload) { - this.MemoryAllocator.BufferCapacityInBytes = 10_000; + this.TestMemoryAllocator.BufferCapacityInBytes = 10_000; using Buffer2D buffer = useSizeOverload ? - this.MemoryAllocator.Allocate2D( + this.TestMemoryAllocator.Allocate2D( new Size(200, 200), preferContiguousImageBuffers: true) : - this.MemoryAllocator.Allocate2D( + this.TestMemoryAllocator.Allocate2D( 200, 200, preferContiguousImageBuffers: true); @@ -85,9 +85,9 @@ public void Construct_PreferContiguousImageBuffers_AllocatesContiguousRegardless [InlineData(50, 10, 20, 4)] public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int alignmentMultiplier) { - this.MemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; + this.TestMemoryAllocator.BufferCapacityInBytes = sizeof(int) * bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); + using Buffer2D buffer = this.TestMemoryAllocator.Allocate2DOverAligned(width, height, alignmentMultiplier); MemoryGroup memoryGroup = buffer.FastMemoryGroup; int expectedAlignment = width * alignmentMultiplier; @@ -97,7 +97,7 @@ public void Allocate2DOveraligned(int bufferCapacity, int width, int height, int [Fact] public void CreateClean() { - using (Buffer2D buffer = this.MemoryAllocator.Allocate2D(42, 42, AllocationOptions.Clean)) + using (Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(42, 42, AllocationOptions.Clean)) { Span span = buffer.DangerousGetSingleSpan(); for (int j = 0; j < span.Length; j++) @@ -117,9 +117,9 @@ public void CreateClean() [InlineData(200, 100, 30, 4, 2)] public unsafe void DangerousGetRowSpan_TestAllocator(int bufferCapacity, int width, int height, int y, int expectedBufferIndex) { - this.MemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; + this.TestMemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; - using (Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height)) + using (Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(width, height)) { Span span = buffer.DangerousGetRowSpan(y); @@ -194,8 +194,8 @@ public unsafe void DangerousGetRowSpan_UnmanagedAllocator(int width, int height) [InlineData(30, 4, 1, -1)] public void TryGetPaddedRowSpanY(int bufferCapacity, int y, int padding, int expectedBufferIndex) { - this.MemoryAllocator.BufferCapacityInBytes = bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2D(3, 5); + this.TestMemoryAllocator.BufferCapacityInBytes = bufferCapacity; + using Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(3, 5); bool expectSuccess = expectedBufferIndex >= 0; bool success = buffer.DangerousTryGetPaddedRowSpan(y, padding, out Span paddedSpan); @@ -219,8 +219,8 @@ public void TryGetPaddedRowSpanY(int bufferCapacity, int y, int padding, int exp [MemberData(nameof(GetRowSpanY_OutOfRange_Data))] public void GetRowSpan_OutOfRange(int bufferCapacity, int width, int height, int y) { - this.MemoryAllocator.BufferCapacityInBytes = bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height); + this.TestMemoryAllocator.BufferCapacityInBytes = bufferCapacity; + using Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(width, height); Exception ex = Assert.ThrowsAny(() => buffer.DangerousGetRowSpan(y)); Assert.True(ex is ArgumentOutOfRangeException || ex is IndexOutOfRangeException); @@ -242,8 +242,8 @@ public void GetRowSpan_OutOfRange(int bufferCapacity, int width, int height, int [MemberData(nameof(Indexer_OutOfRange_Data))] public void Indexer_OutOfRange(int bufferCapacity, int width, int height, int x, int y) { - this.MemoryAllocator.BufferCapacityInBytes = bufferCapacity; - using Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height); + this.TestMemoryAllocator.BufferCapacityInBytes = bufferCapacity; + using Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(width, height); Exception ex = Assert.ThrowsAny(() => buffer[x, y]++); Assert.True(ex is ArgumentOutOfRangeException || ex is IndexOutOfRangeException); @@ -257,9 +257,9 @@ public void Indexer_OutOfRange(int bufferCapacity, int width, int height, int x, [InlineData(500, 200, 30, 199, 29)] public unsafe void Indexer(int bufferCapacity, int width, int height, int x, int y) { - this.MemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; + this.TestMemoryAllocator.BufferCapacityInBytes = sizeof(TestStructs.Foo) * bufferCapacity; - using (Buffer2D buffer = this.MemoryAllocator.Allocate2D(width, height)) + using (Buffer2D buffer = this.TestMemoryAllocator.Allocate2D(width, height)) { int bufferIndex = (width * y) / buffer.FastMemoryGroup.BufferLength; int subBufferStart = (width * y) - (bufferIndex * buffer.FastMemoryGroup.BufferLength); @@ -284,7 +284,7 @@ public unsafe void Indexer(int bufferCapacity, int width, int height, int x, int public void CopyColumns(int width, int height, int startIndex, int destIndex, int columnCount) { var rnd = new Random(123); - using (Buffer2D b = this.MemoryAllocator.Allocate2D(width, height)) + using (Buffer2D b = this.TestMemoryAllocator.Allocate2D(width, height)) { rnd.RandomFill(b.DangerousGetSingleSpan(), 0, 1); @@ -306,7 +306,7 @@ public void CopyColumns(int width, int height, int startIndex, int destIndex, in public void CopyColumns_InvokeMultipleTimes() { var rnd = new Random(123); - using (Buffer2D b = this.MemoryAllocator.Allocate2D(100, 100)) + using (Buffer2D b = this.TestMemoryAllocator.Allocate2D(100, 100)) { rnd.RandomFill(b.DangerousGetSingleSpan(), 0, 1); @@ -328,8 +328,8 @@ public void CopyColumns_InvokeMultipleTimes() [Fact] public void PublicMemoryGroup_IsMemoryGroupView() { - using Buffer2D buffer1 = this.MemoryAllocator.Allocate2D(10, 10); - using Buffer2D buffer2 = this.MemoryAllocator.Allocate2D(10, 10); + using Buffer2D buffer1 = this.TestMemoryAllocator.Allocate2D(10, 10); + using Buffer2D buffer2 = this.TestMemoryAllocator.Allocate2D(10, 10); IMemoryGroup mgBefore = buffer1.MemoryGroup; Buffer2D.SwapOrCopyContent(buffer1, buffer2); @@ -338,21 +338,24 @@ public void PublicMemoryGroup_IsMemoryGroupView() Assert.NotSame(mgBefore, buffer1.MemoryGroup); } + public static TheoryData InvalidLengths { get; set; } = new() + { + { new(-1, -1) }, + { MemoryAllocator.DefaultMaxAllocatableSize2DInBytes + new Size(1, 1) } + }; + [Theory] - [InlineData(-1)] - [InlineData(ushort.MaxValue + 1)] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate2D(length, length)); + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(Size size) + => Assert.Throws(() => this.TestMemoryAllocator.Allocate2D(size.Width, size.Height)); [Theory] - [InlineData(-1)] - [InlineData(ushort.MaxValue + 1)] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_Size(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(length, length))); + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_Size(Size size) + => Assert.Throws(() => this.TestMemoryAllocator.Allocate2D(new Size(size))); [Theory] - [InlineData(-1)] - [InlineData(ushort.MaxValue + 1)] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_OverAligned(int length) - => Assert.Throws(() => this.MemoryAllocator.Allocate2DOverAligned(length, length, 1)); + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_OverAligned(Size size) + => Assert.Throws(() => this.TestMemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); } From 9c7d6dd0925c0b6263332d62c30d40007ed0dbe0 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 23 Mar 2024 22:03:26 +1000 Subject: [PATCH 6/8] OCD --- src/ImageSharp/Memory/Allocators/MemoryAllocator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index ac1f3866b7..53b6362f38 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -135,7 +135,7 @@ private static int GetDefaultMaxAllocatableSize1DInBytes() // It's possible to require buffers that are not related to image dimensions. // For example, when we need to allocate buffers for IDAT chunks in PNG files or when allocating // cache buffers for image quantization. - // Limit the maximum buffer size to 64MB for 64bit processes and 32MB for 32 bit processes. + // Limit the maximum buffer size to 64MB for 64-bit processes and 32MB for 32-bit processes. int limitInMB = Environment.Is64BitProcess ? 64 : 32; return limitInMB * 1024 * 1024; } From 5fb0f99def8ba5f57749936bd78c97ae7b19116b Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sun, 24 Mar 2024 00:36:10 +1000 Subject: [PATCH 7/8] Fix 2D limit, improve message and simplify --- .../Memory/Allocators/MemoryAllocator.cs | 34 ++++++++++++++++--- .../Allocators/SimpleGcMemoryAllocator.cs | 2 +- ...iformUnmanagedMemoryPoolMemoryAllocator.cs | 2 +- .../Allocators/UnmanagedMemoryAllocator.cs | 2 +- .../Memory/MemoryAllocatorExtensions.cs | 9 ++--- 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 53b6362f38..eda0456d4d 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -101,7 +101,31 @@ internal virtual MemoryGroup AllocateGroup( where T : struct => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); - internal static void MemoryGuardMustBeBetweenOrEqualTo(int value, int min, int max, string paramName) + internal void MemoryGuardAllocation2D(Size value, string paramName) + where T : struct + { + int typeSizeInBytes = Unsafe.SizeOf(); + long widthInBytes = value.Width * typeSizeInBytes; + + // If a sufficiently large value is passed in, the multiplication will overflow. + // We can detect this by checking if the result is less than the original value. + if (widthInBytes < value.Width && value.Width > 0) + { + widthInBytes = long.MaxValue; + } + + int maxWidth = this.MaxAllocatableSize2DInBytes.Width; + int maxHeight = this.MaxAllocatableSize2DInBytes.Height; + if (widthInBytes >= 0 && widthInBytes <= maxWidth && value.Height >= 0 && value.Height <= maxHeight) + { + return; + } + + throw new InvalidMemoryOperationException( + $"An allocation was attempted that exceeded allowable limits; \"{paramName}\" must be less than or equal {maxWidth}x{maxHeight}, was {widthInBytes}x{value.Height}"); + } + + internal void MemoryGuardAllocation1D(int value, string paramName) where T : struct { int typeSizeInBytes = Unsafe.SizeOf(); @@ -114,20 +138,20 @@ internal static void MemoryGuardMustBeBetweenOrEqualTo(int value, int min, in valueInBytes = long.MaxValue; } - if (valueInBytes >= min && valueInBytes <= max) + if (valueInBytes >= 0 && valueInBytes <= this.MaxAllocatableSize1DInBytes) { return; } - throw new InvalidMemoryOperationException($"Parameter \"{paramName}\" must be between or equal to {min} and {max}, was {valueInBytes}"); + throw new InvalidMemoryOperationException( + $"An allocation was attempted that exceeded allowable limits; \"{paramName}\" must be less than or equal {this.MaxAllocatableSize1DInBytes}, was {valueInBytes}"); } private static Size GetDefaultMaxAllocatableSize2DInBytes() { // Limit dimensions to 65535x65535 and 32767x32767 @ 4 bytes per pixel for 64 and 32 bit processes respectively. int maxLength = Environment.Is64BitProcess ? ushort.MaxValue : short.MaxValue; - int maxLengthInRgba32Bytes = maxLength * Unsafe.SizeOf(); - return new(maxLengthInRgba32Bytes, maxLengthInRgba32Bytes); + return new(maxLength * Unsafe.SizeOf(), maxLength); } private static int GetDefaultMaxAllocatableSize1DInBytes() diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index 85d2ce9c58..e46ddb4501 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -17,7 +17,7 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1DInBytes, nameof(length)); + this.MemoryGuardAllocation1D(length, nameof(length)); return new BasicArrayBuffer(new T[length]); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index fefe8b6688..47cfaad351 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -81,7 +81,7 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1DInBytes, nameof(length)); + this.MemoryGuardAllocation1D(length, nameof(length)); int lengthInBytes = length * Unsafe.SizeOf(); if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) diff --git a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs index 1484cc7329..99a6e5b1ff 100644 --- a/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UnmanagedMemoryAllocator.cs @@ -20,7 +20,7 @@ internal class UnmanagedMemoryAllocator : MemoryAllocator public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - MemoryGuardMustBeBetweenOrEqualTo(length, 0, this.MaxAllocatableSize1DInBytes, nameof(length)); + this.MemoryGuardAllocation1D(length, nameof(length)); UnmanagedBuffer buffer = UnmanagedBuffer.Allocate(length); if (options.Has(AllocationOptions.Clean)) diff --git a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs index af84945abf..add27ea717 100644 --- a/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs +++ b/src/ImageSharp/Memory/MemoryAllocatorExtensions.cs @@ -29,8 +29,7 @@ public static Buffer2D Allocate2D( AllocationOptions options = AllocationOptions.None) where T : struct { - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Width, nameof(width)); - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Height, nameof(height)); + memoryAllocator.MemoryGuardAllocation2D(new Size(width, height), "size"); long groupLength = (long)width * height; MemoryGroup memoryGroup; @@ -107,8 +106,7 @@ internal static Buffer2D Allocate2DOverAligned( AllocationOptions options = AllocationOptions.None) where T : struct { - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(width, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Width, nameof(width)); - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(height, 0, memoryAllocator.MaxAllocatableSize2DInBytes.Height, nameof(height)); + memoryAllocator.MemoryGuardAllocation2D(new Size(width, height), "size"); long groupLength = (long)width * height; MemoryGroup memoryGroup = memoryAllocator.AllocateGroup( @@ -133,8 +131,7 @@ internal static IMemoryOwner AllocatePaddedPixelRowBuffer( int paddingInBytes) { int length = (width * pixelSizeInBytes) + paddingInBytes; - MemoryAllocator.MemoryGuardMustBeBetweenOrEqualTo(length, 0, memoryAllocator.MaxAllocatableSize1DInBytes, nameof(length)); - + memoryAllocator.MemoryGuardAllocation1D(length, nameof(length)); return memoryAllocator.Allocate(length); } } From 1eecd402f78d89fd5b4ecf8f8b5d4546ce9e9893 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 27 Mar 2024 15:22:53 +1000 Subject: [PATCH 8/8] Fix assignment limits --- .../Memory/Allocators/MemoryAllocator.cs | 44 ++++++++----------- .../Allocators/SimpleGcMemoryAllocator.cs | 2 +- .../Formats/Bmp/BmpEncoderTests.cs | 6 +-- .../Formats/Jpg/JpegDecoderTests.cs | 2 +- .../ImageSharp.Tests/Memory/Buffer2DTests.cs | 16 ++++--- 5 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index eda0456d4d..6a2cf79c7c 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -20,7 +20,7 @@ public abstract class MemoryAllocator /// /// Gets the default max allocatable size of a 2D buffer in bytes. /// - public static readonly Size DefaultMaxAllocatableSize2DInBytes = GetDefaultMaxAllocatableSize2DInBytes(); + public static readonly ulong DefaultMaxAllocatableSize2DInBytes = GetDefaultMaxAllocatableSize2DInBytes(); /// /// Gets the default platform-specific global instance that @@ -36,7 +36,7 @@ public abstract class MemoryAllocator /// Gets or sets the maximum allowable allocatable size of a 2 dimensional buffer. /// Defaults to . /// - public Size MaxAllocatableSize2DInBytes { get; set; } = DefaultMaxAllocatableSize2DInBytes; + public ulong MaxAllocatableSize2DInBytes { get; set; } = DefaultMaxAllocatableSize2DInBytes; /// /// Gets or sets the maximum allowable allocatable size of a 1 dimensional buffer. @@ -104,41 +104,35 @@ internal virtual MemoryGroup AllocateGroup( internal void MemoryGuardAllocation2D(Size value, string paramName) where T : struct { - int typeSizeInBytes = Unsafe.SizeOf(); - long widthInBytes = value.Width * typeSizeInBytes; - - // If a sufficiently large value is passed in, the multiplication will overflow. - // We can detect this by checking if the result is less than the original value. - if (widthInBytes < value.Width && value.Width > 0) + if (value.Width < 0 || value.Height < 0) { - widthInBytes = long.MaxValue; + throw new InvalidMemoryOperationException($"An allocation was attempted that exceeded allowable limits; \"{paramName}\" at {value.Width}x{value.Height}"); } - int maxWidth = this.MaxAllocatableSize2DInBytes.Width; - int maxHeight = this.MaxAllocatableSize2DInBytes.Height; - if (widthInBytes >= 0 && widthInBytes <= maxWidth && value.Height >= 0 && value.Height <= maxHeight) + ulong typeSizeInBytes = (ulong)Unsafe.SizeOf(); + ulong valueInBytes = (ulong)value.Width * typeSizeInBytes * (ulong)value.Height; + + if (valueInBytes <= this.MaxAllocatableSize2DInBytes) { return; } throw new InvalidMemoryOperationException( - $"An allocation was attempted that exceeded allowable limits; \"{paramName}\" must be less than or equal {maxWidth}x{maxHeight}, was {widthInBytes}x{value.Height}"); + $"An allocation was attempted that exceeded allowable limits; \"{paramName}\" at {value.Width}x{value.Height} must be less than or equal to {this.MaxAllocatableSize2DInBytes}, was {valueInBytes}"); } internal void MemoryGuardAllocation1D(int value, string paramName) where T : struct { - int typeSizeInBytes = Unsafe.SizeOf(); - long valueInBytes = value * typeSizeInBytes; - - // If a sufficiently large value is passed in, the multiplication will overflow. - // We can detect this by checking if the result is less than the original value. - if (valueInBytes < value && value > 0) + if (value < 0) { - valueInBytes = long.MaxValue; + throw new InvalidMemoryOperationException($"An allocation was attempted that exceeded allowable limits; {paramName} must be greater than or equal to zero, was {value}"); } - if (valueInBytes >= 0 && valueInBytes <= this.MaxAllocatableSize1DInBytes) + ulong typeSizeInBytes = (ulong)Unsafe.SizeOf(); + ulong valueInBytes = (ulong)value * typeSizeInBytes; + + if (valueInBytes <= (ulong)this.MaxAllocatableSize1DInBytes) { return; } @@ -147,11 +141,11 @@ internal void MemoryGuardAllocation1D(int value, string paramName) $"An allocation was attempted that exceeded allowable limits; \"{paramName}\" must be less than or equal {this.MaxAllocatableSize1DInBytes}, was {valueInBytes}"); } - private static Size GetDefaultMaxAllocatableSize2DInBytes() + private static ulong GetDefaultMaxAllocatableSize2DInBytes() { - // Limit dimensions to 65535x65535 and 32767x32767 @ 4 bytes per pixel for 64 and 32 bit processes respectively. - int maxLength = Environment.Is64BitProcess ? ushort.MaxValue : short.MaxValue; - return new(maxLength * Unsafe.SizeOf(), maxLength); + // Limit dimensions to 32767x32767 and 16383x16383 @ 4 bytes per pixel for 64 and 32 bit processes respectively. + ulong maxLength = Environment.Is64BitProcess ? ushort.MaxValue / 2 : (ulong)short.MaxValue / 4; + return maxLength * (ulong)Unsafe.SizeOf() * maxLength; } private static int GetDefaultMaxAllocatableSize1DInBytes() diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index e46ddb4501..a5f7499ed4 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Memory; public sealed class SimpleGcMemoryAllocator : MemoryAllocator { /// - protected internal override int GetBufferCapacityInBytes() => int.MaxValue; + protected internal override int GetBufferCapacityInBytes() => this.MaxAllocatableSize1DInBytes; /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs index 12bc547a75..00cf0b984a 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs @@ -351,8 +351,8 @@ public void Encode_PreservesColorProfile(TestImageProvider provi public static TheoryData Encode_WorksWithSizeGreaterThen65k_Data { get; set; } = new() { - { new Size(1, MemoryAllocator.DefaultMaxAllocatableSize2DInBytes.Height + 1) }, - { new Size(MemoryAllocator.DefaultMaxAllocatableSize2DInBytes.Width + 1, 1) } + { new Size(65535, 1) }, + { new Size(1, 65535) }, }; [Theory] @@ -362,7 +362,7 @@ public void Encode_WorksWithSizeGreaterThen65k(Size size) Exception exception = Record.Exception(() => { Configuration configuration = Configuration.CreateDefaultInstance(); - configuration.MemoryAllocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null) { MaxAllocatableSize2DInBytes = size + new Size(1, 1) }; + configuration.MemoryAllocator = new UniformUnmanagedMemoryPoolMemoryAllocator(null) { MaxAllocatableSize2DInBytes = ulong.MaxValue }; using Image image = new Image(configuration, size.Width, size.Height); using MemoryStream memStream = new(); image.Save(memStream, BmpEncoder); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs index 972cabe242..dbfd1c1c20 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.cs @@ -338,7 +338,7 @@ public void Issue2564_DecodeWorks(TestImageProvider provider) } [Theory] - [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.L8)] + [WithFile(TestImages.Jpeg.Issues.HangBadScan, PixelTypes.Rgba32)] public void DecodeHang_ThrowsException(TestImageProvider provider) where TPixel : unmanaged, IPixel => Assert.Throws(() => { using Image image = provider.GetImage(JpegDecoder.Instance); }); diff --git a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs index cecd79c2e6..00c09b87a6 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -4,6 +4,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; // ReSharper disable InconsistentNaming namespace SixLabors.ImageSharp.Tests.Memory; @@ -341,21 +342,22 @@ public void PublicMemoryGroup_IsMemoryGroupView() public static TheoryData InvalidLengths { get; set; } = new() { { new(-1, -1) }, - { MemoryAllocator.DefaultMaxAllocatableSize2DInBytes + new Size(1, 1) } + { new(32768, 32767) }, + { new(32767, 32768) } }; [Theory] [MemberData(nameof(InvalidLengths))] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(Size size) - => Assert.Throws(() => this.TestMemoryAllocator.Allocate2D(size.Width, size.Height)); + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(Size size) + => Assert.Throws(() => this.TestMemoryAllocator.Allocate2D(size.Width, size.Height)); [Theory] [MemberData(nameof(InvalidLengths))] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_Size(Size size) - => Assert.Throws(() => this.TestMemoryAllocator.Allocate2D(new Size(size))); + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_Size(Size size) + => Assert.Throws(() => this.TestMemoryAllocator.Allocate2D(new Size(size))); [Theory] [MemberData(nameof(InvalidLengths))] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException_OverAligned(Size size) - => Assert.Throws(() => this.TestMemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) + => Assert.Throws(() => this.TestMemoryAllocator.Allocate2DOverAligned(size.Width, size.Height, 1)); }