Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit c65a220

Browse files
Fix incorrect bounds check in ArrayMemoryPoolBuffer.Pin (#28032)
1 parent 52da166 commit c65a220

File tree

3 files changed

+42
-9
lines changed

3 files changed

+42
-9
lines changed

src/System.Memory/src/System/Buffers/ArrayMemoryPool.ArrayMemoryPoolBuffer.cs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,22 @@ public sealed override MemoryHandle Pin(int byteOffset = 0)
7272
{
7373
Retain(); // this checks IsDisposed
7474

75-
if (byteOffset != 0 && (((uint)byteOffset) - 1) / Unsafe.SizeOf<T>() >= _array.Length)
76-
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.byteOffset);
75+
try
76+
{
77+
if ((IntPtr.Size == 4 && (uint)byteOffset > (uint)_array.Length * (uint)Unsafe.SizeOf<T>())
78+
|| (IntPtr.Size != 4 && (ulong)byteOffset > (uint)_array.Length * (ulong)Unsafe.SizeOf<T>()))
79+
{
80+
ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.byteOffset);
81+
}
7782

78-
GCHandle handle = GCHandle.Alloc(_array, GCHandleType.Pinned);
79-
return new MemoryHandle(this, ((byte*)handle.AddrOfPinnedObject()) + byteOffset, handle);
83+
GCHandle handle = GCHandle.Alloc(_array, GCHandleType.Pinned);
84+
return new MemoryHandle(this, ((byte*)handle.AddrOfPinnedObject()) + byteOffset, handle);
85+
}
86+
catch
87+
{
88+
Release();
89+
throw;
90+
}
8091
}
8192
}
8293

src/System.Memory/tests/Memory/CustomMemoryForTest.cs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,24 @@ public override MemoryHandle Pin(int byteOffset = 0)
5151
{
5252
unsafe
5353
{
54-
Retain();
55-
if (byteOffset != 0 && (((uint)byteOffset) - 1) / Unsafe.SizeOf<T>() >= _array.Length)
56-
throw new ArgumentOutOfRangeException(nameof(byteOffset));
57-
var handle = GCHandle.Alloc(_array, GCHandleType.Pinned);
58-
return new MemoryHandle(this, Unsafe.Add<byte>((void*)handle.AddrOfPinnedObject(), _offset + byteOffset), handle);
54+
Retain(); // this checks IsDisposed
55+
56+
try
57+
{
58+
if ((IntPtr.Size == 4 && (uint)byteOffset > (uint)_array.Length * (uint)Unsafe.SizeOf<T>())
59+
|| (IntPtr.Size != 4 && (ulong)byteOffset > (uint)_array.Length * (ulong)Unsafe.SizeOf<T>()))
60+
{
61+
throw new ArgumentOutOfRangeException(nameof(byteOffset));
62+
}
63+
64+
var handle = GCHandle.Alloc(_array, GCHandleType.Pinned);
65+
return new MemoryHandle(this, Unsafe.Add<byte>((void*)handle.AddrOfPinnedObject(), _offset + byteOffset), handle);
66+
}
67+
catch
68+
{
69+
Release();
70+
throw;
71+
}
5972
}
6073
}
6174

src/System.Memory/tests/Memory/OwnedMemory.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,15 @@ public static void OwnedMemoryPinArray()
117117
handle.Dispose();
118118
}
119119

120+
[Fact]
121+
[OuterLoop]
122+
public static void OwnedMemoryPinLargeArray()
123+
{
124+
int[] array = new int[0x2000_0000]; // will produce array with total byte length > 2 GB
125+
OwnedMemory<int> owner = new CustomMemoryForTest<int>(array);
126+
Assert.Throws<ArgumentOutOfRangeException>(() => owner.Pin(int.MinValue));
127+
}
128+
120129
[Fact]
121130
public static void MemoryFromOwnedMemoryAfterDispose()
122131
{

0 commit comments

Comments
 (0)