diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 316f9385eb..d6c3256e06 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1504,6 +1504,9 @@ private void SkipChunkDataAndCrc(in PngChunk chunk) 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/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs index 59d4d5bda4..df8dfbe0a0 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs @@ -14,7 +14,7 @@ namespace SixLabors.ImageSharp.Memory.Internals internal struct UnmanagedMemoryHandle : IEquatable { // Number of allocation re-attempts when detecting OutOfMemoryException. - private const int MaxAllocationAttempts = 1000; + private const int MaxAllocationAttempts = 10; // Track allocations for testing purposes: private static int totalOutstandingHandles; diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs index 4df78d9d93..d064c1fc45 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocator.cs @@ -3,6 +3,8 @@ using System; using System.Buffers; +using System.Collections.Generic; +using System.Runtime.CompilerServices; namespace SixLabors.ImageSharp.Memory { @@ -11,6 +13,8 @@ namespace SixLabors.ImageSharp.Memory /// public abstract class MemoryAllocator { + private const int OneGigabyte = 1 << 30; + /// /// Gets the default platform-specific global instance that /// serves as the default value for . @@ -21,6 +25,10 @@ public abstract class MemoryAllocator /// public static MemoryAllocator Default { get; } = Create(); + internal long MemoryGroupAllocationLimitBytes { get; private set; } = Environment.Is64BitProcess ? 4L * OneGigabyte : OneGigabyte; + + internal int SingleBufferAllocationLimitBytes { get; private set; } = OneGigabyte; + /// /// Gets the length of the largest contiguous buffer that can be handled by this allocator instance in bytes. /// @@ -31,16 +39,24 @@ public abstract class MemoryAllocator /// Creates a default instance of a optimized for the executing platform. /// /// The . - public static MemoryAllocator Create() => - new UniformUnmanagedMemoryPoolMemoryAllocator(null); + public static MemoryAllocator Create() => Create(default); /// /// Creates the default using the provided options. /// /// The . /// The . - public static MemoryAllocator Create(MemoryAllocatorOptions options) => - new UniformUnmanagedMemoryPoolMemoryAllocator(options.MaximumPoolSizeMegabytes); + public static MemoryAllocator Create(MemoryAllocatorOptions options) + { + UniformUnmanagedMemoryPoolMemoryAllocator allocator = new(options.MaximumPoolSizeMegabytes); + if (options.AllocationLimitMegabytes.HasValue) + { + allocator.MemoryGroupAllocationLimitBytes = options.AllocationLimitMegabytes.Value * 1024 * 1024; + allocator.SingleBufferAllocationLimitBytes = (int)Math.Min(allocator.SingleBufferAllocationLimitBytes, allocator.MemoryGroupAllocationLimitBytes); + } + + return allocator; + } /// /// Allocates an , holding a of length . @@ -65,16 +81,35 @@ public virtual void ReleaseRetainedResources() /// /// Allocates a . /// + /// The type of element to allocate. /// The total length of the buffer. /// The expected alignment (eg. to make sure image rows fit into single buffers). /// The . /// A new . /// Thrown when 'blockAlignment' converted to bytes is greater than the buffer capacity of the allocator. - internal virtual MemoryGroup AllocateGroup( + internal MemoryGroup AllocateGroup( long totalLength, int bufferAlignment, AllocationOptions options = AllocationOptions.None) where T : struct - => MemoryGroup.Allocate(this, totalLength, bufferAlignment, options); + { + if (totalLength < 0) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={totalLength}."); + } + + ulong totalLengthInBytes = (ulong)totalLength * (ulong)Unsafe.SizeOf(); + if (totalLengthInBytes > (ulong)this.MemoryGroupAllocationLimitBytes) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={totalLengthInBytes} that exceeded the limit {this.MemoryGroupAllocationLimitBytes}."); + } + + // Cast to long is safe because we already checked that the total length is within the limit. + return this.AllocateGroupCore(totalLength, (long)totalLengthInBytes, bufferAlignment, options); + } + + internal virtual MemoryGroup AllocateGroupCore(long totalLengthInElements, long totalLengthInBytes, int bufferAlignment, AllocationOptions options) + where T : struct + => MemoryGroup.Allocate(this, totalLengthInElements, bufferAlignment, options); } } diff --git a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs index 22a0410755..70dc515fe4 100644 --- a/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs +++ b/src/ImageSharp/Memory/Allocators/MemoryAllocatorOptions.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. namespace SixLabors.ImageSharp.Memory @@ -9,6 +9,7 @@ namespace SixLabors.ImageSharp.Memory public struct MemoryAllocatorOptions { private int? maximumPoolSizeMegabytes; + private int? allocationLimitMegabytes; /// /// Gets or sets a value defining the maximum size of the 's internal memory pool @@ -27,5 +28,23 @@ public int? MaximumPoolSizeMegabytes this.maximumPoolSizeMegabytes = value; } } + + /// + /// Gets or sets a value defining the maximum (discontiguous) buffer size that can be allocated by the allocator in Megabytes. + /// means platform default: 1GB on 32-bit processes, 4GB on 64-bit processes. + /// + public int? AllocationLimitMegabytes + { + readonly get => this.allocationLimitMegabytes; + set + { + if (value.HasValue) + { + Guard.MustBeGreaterThan(value.Value, 0, nameof(this.AllocationLimitMegabytes)); + } + + this.allocationLimitMegabytes = value; + } + } } } diff --git a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs index a53ecbc66e..e2b132139b 100644 --- a/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/SimpleGcMemoryAllocator.cs @@ -1,7 +1,8 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System.Buffers; +using System.Runtime.CompilerServices; using SixLabors.ImageSharp.Memory.Internals; namespace SixLabors.ImageSharp.Memory @@ -17,7 +18,16 @@ public sealed class SimpleGcMemoryAllocator : MemoryAllocator /// public override IMemoryOwner Allocate(int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); + if (length < 0) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}."); + } + + ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf(); + if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={lengthInBytes} that exceeded the limit {this.SingleBufferAllocationLimitBytes}."); + } return new BasicArrayBuffer(new T[length]); } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 7395ec9ac7..a8056db537 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -87,10 +87,18 @@ public override IMemoryOwner Allocate( int length, AllocationOptions options = AllocationOptions.None) { - Guard.MustBeGreaterThanOrEqualTo(length, 0, nameof(length)); - int lengthInBytes = length * Unsafe.SizeOf(); + if (length < 0) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={length}."); + } + + ulong lengthInBytes = (ulong)length * (ulong)Unsafe.SizeOf(); + if (lengthInBytes > (ulong)this.SingleBufferAllocationLimitBytes) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of length={lengthInBytes} that exceeded the limit {this.SingleBufferAllocationLimitBytes}."); + } - if (lengthInBytes <= this.sharedArrayPoolThresholdInBytes) + if (lengthInBytes <= (ulong)this.sharedArrayPoolThresholdInBytes) { var buffer = new SharedArrayPoolBuffer(length); if (options.Has(AllocationOptions.Clean)) @@ -101,7 +109,7 @@ public override IMemoryOwner Allocate( return buffer; } - if (lengthInBytes <= this.poolBufferSizeInBytes) + if (lengthInBytes <= (ulong)this.poolBufferSizeInBytes) { UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) @@ -115,20 +123,15 @@ public override IMemoryOwner Allocate( } /// - internal override MemoryGroup AllocateGroup( - long totalLength, + internal override MemoryGroup AllocateGroupCore( + long totalLengthInElements, + long totalLengthInBytes, int bufferAlignment, AllocationOptions options = AllocationOptions.None) { - long totalLengthInBytes = totalLength * Unsafe.SizeOf(); - if (totalLengthInBytes < 0) - { - throw new InvalidMemoryOperationException("Attempted to allocate a MemoryGroup of a size that is not representable."); - } - if (totalLengthInBytes <= this.sharedArrayPoolThresholdInBytes) { - var buffer = new SharedArrayPoolBuffer((int)totalLength); + var buffer = new SharedArrayPoolBuffer((int)totalLengthInElements); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } @@ -138,18 +141,18 @@ internal override MemoryGroup AllocateGroup( UnmanagedMemoryHandle mem = this.pool.Rent(); if (mem.IsValid) { - UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLength, options.Has(AllocationOptions.Clean)); + UnmanagedBuffer buffer = this.pool.CreateGuardedBuffer(mem, (int)totalLengthInElements, options.Has(AllocationOptions.Clean)); return MemoryGroup.CreateContiguous(buffer, options.Has(AllocationOptions.Clean)); } } // Attempt to rent the whole group from the pool, allocate a group of unmanaged buffers if the attempt fails: - if (MemoryGroup.TryAllocate(this.pool, totalLength, bufferAlignment, options, out MemoryGroup poolGroup)) + if (MemoryGroup.TryAllocate(this.pool, totalLengthInElements, bufferAlignment, options, out MemoryGroup? poolGroup)) { return poolGroup; } - return MemoryGroup.Allocate(this.nonPoolAllocator, totalLength, bufferAlignment, options); + return MemoryGroup.Allocate(this.nonPoolAllocator, totalLengthInElements, bufferAlignment, options); } public override void ReleaseRetainedResources() => this.pool.Release(); diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index 9844f4a3bc..d77c2c4dc0 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs @@ -20,7 +20,7 @@ namespace SixLabors.ImageSharp.Memory /// The element type. internal abstract partial class MemoryGroup : IMemoryGroup, IDisposable where T : struct - { + { private static readonly int ElementSize = Unsafe.SizeOf(); private MemoryGroupSpanCache memoryGroupSpanCache; @@ -43,7 +43,7 @@ private MemoryGroup(int bufferLength, long totalLength) /// public bool IsValid { get; private set; } = true; - public MemoryGroupView View { get; private set; } + public MemoryGroupView View { get; private set; } = null!; /// public abstract Memory this[int index] { get; } @@ -85,12 +85,14 @@ public static MemoryGroup Allocate( { int bufferCapacityInBytes = allocator.GetBufferCapacityInBytes(); Guard.NotNull(allocator, nameof(allocator)); - Guard.MustBeGreaterThanOrEqualTo(totalLengthInElements, 0, nameof(totalLengthInElements)); - Guard.MustBeGreaterThanOrEqualTo(bufferAlignmentInElements, 0, nameof(bufferAlignmentInElements)); - int blockCapacityInElements = bufferCapacityInBytes / ElementSize; + if (totalLengthInElements < 0) + { + throw new InvalidMemoryOperationException($"Attempted to allocate a buffer of negative length={totalLengthInElements}."); + } - if (bufferAlignmentInElements > blockCapacityInElements) + int blockCapacityInElements = bufferCapacityInBytes / ElementSize; + if (bufferAlignmentInElements < 0 || bufferAlignmentInElements > blockCapacityInElements) { throw new InvalidMemoryOperationException( $"The buffer capacity of the provided MemoryAllocator is insufficient for the requested buffer alignment: {bufferAlignmentInElements}."); diff --git a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs index 92b1d8d359..e9730fb964 100644 --- a/src/ImageSharp/Memory/InvalidMemoryOperationException.cs +++ b/src/ImageSharp/Memory/InvalidMemoryOperationException.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System; +using System.Diagnostics.CodeAnalysis; namespace SixLabors.ImageSharp.Memory { diff --git a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs index 43ec45a34f..5576eac993 100644 --- a/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Bmp/BmpDecoderTests.cs @@ -619,5 +619,14 @@ public void BmpDecoder_CanDecode_Os2BitmapArray(TestImageProvider(TestImageProvider provider) + where TPixel : unmanaged, IPixel + => Assert.Throws(() => + { + using Image image = provider.GetImage(BmpDecoder); + }); } } diff --git a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs index 3ab45be82d..c4ff74cdd2 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/SimpleGcMemoryAllocatorTests.cs @@ -21,13 +21,17 @@ public BufferTests() protected SimpleGcMemoryAllocator MemoryAllocator { get; } = new SimpleGcMemoryAllocator(); - [Theory] - [InlineData(-1)] - public void Allocate_IncorrectAmount_ThrowsCorrect_ArgumentOutOfRangeException(int length) + public static TheoryData InvalidLengths { get; set; } = new() { - ArgumentOutOfRangeException ex = Assert.Throws(() => this.MemoryAllocator.Allocate(length)); - Assert.Equal("length", ex.ParamName); - } + { -1 }, + { (1 << 30) + 1 } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length) + => 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 f49dcf05db..6123e1b491 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -118,6 +118,17 @@ public void AllocateGroup_SizeInBytesOverLongMaxValue_ThrowsInvalidMemoryOperati Assert.Throws(() => allocator.AllocateGroup(int.MaxValue * (long)int.MaxValue, int.MaxValue)); } + public static TheoryData InvalidLengths { get; set; } = new() + { + { -1 }, + { (1 << 30) + 1 } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(int length) + => 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 486fbe4640..751f35890b 100644 --- a/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs +++ b/tests/ImageSharp.Tests/Memory/Buffer2DTests.cs @@ -7,6 +7,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Memory; +using SixLabors.ImageSharp.PixelFormats; using Xunit; @@ -342,5 +343,27 @@ public void PublicMemoryGroup_IsMemoryGroupView() Assert.False(mgBefore.IsValid); Assert.NotSame(mgBefore, buffer1.MemoryGroup); } + + public static TheoryData InvalidLengths { get; set; } = new() + { + { new(-1, -1) }, + { new(32768, 32769) }, + { new(32769, 32768) } + }; + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(size.Width, size.Height)); + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_Size(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2D(new Size(size))); + + [Theory] + [MemberData(nameof(InvalidLengths))] + public void Allocate_IncorrectAmount_ThrowsCorrect_InvalidMemoryOperationException_OverAligned(Size size) + => Assert.Throws(() => this.MemoryAllocator.Allocate2DOveraligned(size.Width, size.Height, 1)); } } diff --git a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs index adfafcb890..299a6c5134 100644 --- a/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs +++ b/tests/ImageSharp.Tests/Memory/DiscontiguousBuffers/MemoryGroupTests.Allocate.cs @@ -1,8 +1,7 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; -using System.Buffers; using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; @@ -10,7 +9,6 @@ using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Memory.Internals; using Xunit; -using Xunit.Abstractions; namespace SixLabors.ImageSharp.Tests.Memory.DiscontiguousBuffers { @@ -225,12 +223,18 @@ public void MemoryAllocatorIsUtilizedCorrectly(AllocationOptions allocationOptio [StructLayout(LayoutKind.Sequential, Size = 5)] internal struct S5 { - public override string ToString() => "S5"; + public override readonly string ToString() => nameof(S5); } [StructLayout(LayoutKind.Sequential, Size = 4)] internal struct S4 { - public override string ToString() => "S4"; + public override readonly string ToString() => nameof(S4); + } + + [StructLayout(LayoutKind.Explicit, Size = 512)] + internal struct S512 + { + public override readonly string ToString() => nameof(S512); } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index d287ecb4cf..6778f10fce 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -387,6 +387,8 @@ public static class Bmp public const string Rgba321010102 = "Bmp/rgba32-1010102.bmp"; public const string RgbaAlphaBitfields = "Bmp/rgba32abf.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..4a42e456cb --- /dev/null +++ b/tests/Images/Input/Bmp/issue-2696.bmp @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:de7e7ec0454a55f6a76c859f356b240a3d4cb56ca50dfa209a1813dd09e12076 +size 143