Skip to content

fix(serialization): enforce Arc buffer bounds and lifetimes #11474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ReubenBond merged 3 commits into from
Oct 11, 2026
Merged

fix(serialization): enforce Arc buffer bounds and lifetimes #11474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f95c23
fix(serialization): enforce Arc buffer bounds and lifetimes
ReubenBond Oct 10, 2026
ba8d910
fix(serialization): guard disposed Arc reader cursor mutations
ReubenBond Oct 11, 2026
37bbb3a
fix(serialization): reject empty appends to disposed Arc writers
ReubenBond Oct 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 12 additions & 21 deletions src/Orleans.Serialization/Buffers/Adaptors/BufferSliceReaderInput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public struct ArcBufferReaderInput(in ArcBuffer slice)
private ArcBufferPage? _page = slice.First;
private int _position;

internal readonly ArcBufferPage First => _slice.First;
internal readonly ArcBufferPage? First => _slice.First;
internal readonly int Position => _position;
internal readonly int Offset => _slice.Offset;
internal readonly int Length => _slice.Length;
Expand All @@ -138,28 +138,19 @@ internal readonly ArcBufferReaderInput ForkFrom(int position)

internal ReadOnlySpan<byte> GetNext()
{
_slice.CheckValidity();
Debug.Assert(_position <= Length);
if (_page is not null)
if (Length == 0) return default;
while (_page is not null && _position < Length)
{
if (_page == First)
{
Debug.Assert(_position == 0);
var offset = Offset;
var length = Math.Min(Length, _page.Length - offset);
_position += length;
var result = _page.AsSpan(offset, length);
_page = _page.Next;
return result;
}

if (_position != Length)
{
var length = Math.Min(Length - _position, _page.Length);
_position += length;
var result = _page.AsSpan(0, length);
_page = _page.Next;
return result;
}
var page = _page;
var offset = page == First ? Offset : 0;
var length = Math.Min(Length - _position, page.Length - offset);
_page = page.Next;
if (length == 0) continue;

_position += length;
return page.AsSpan(offset, length);
}

ThrowInsufficientData();
Expand Down
181 changes: 137 additions & 44 deletions src/Orleans.Serialization/Buffers/ArcBufferWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,12 @@
/// <summary>
/// Appends the pages referenced by <paramref name="input"/> by pinning them.
/// </summary>
/// <exception cref="ObjectDisposedException">The writer has been disposed.</exception>
public void AppendPinned(ArcBuffer input)
{
if (input.Length == 0)
{
ThrowIfDisposed();
return;
}

Expand All @@ -380,7 +382,7 @@
}

_readPage.Unpin(_readPage.Version);
_readPage = input.First;
_readPage = input.First!;
_writePage = _tail = tail!;
_readIndex = input.Offset;
_totalLength = checked(input.Offset + input.Length);
Expand All @@ -389,18 +391,25 @@

private void ThrowIfPinnedPages()
{
ThrowIfDisposed();

if (_hasPinnedPages)
{
throw new InvalidOperationException("A writer containing pinned pages cannot be mutated.");
}
}

internal bool TryPeek(long offset, out byte value)
private void ThrowIfDisposed()
{
if (_readPage is null)
if (_writePage is null)
{
throw new ObjectDisposedException(nameof(ArcBufferWriter));
}
}

internal bool TryPeek(long offset, out byte value)
{
ThrowIfDisposed();

if (offset < 0)
{
Expand Down Expand Up @@ -469,8 +478,11 @@
/// </summary>
/// <param name="count">The number of bytes to consume.</param>
/// <returns>A slice of unconsumed data.</returns>
/// <exception cref="ObjectDisposedException">The writer has been disposed.</exception>
public ArcBuffer PeekSlice(int count)
{
ThrowIfDisposed();

#if NET6_0_OR_GREATER
ArgumentOutOfRangeException.ThrowIfLessThan(count, 0);
ArgumentOutOfRangeException.ThrowIfGreaterThan(count, UnconsumedLength);
Expand All @@ -481,7 +493,7 @@
Debug.Assert(count >= 0);
Debug.Assert(count <= UnconsumedLength);

var result = new ArcBuffer(_readPage, token: _readPage.Version, offset: _readIndex, count);
var result = new ArcBuffer(_readPage, _readIndex, count);
result.Pin();
return result;
}
Expand All @@ -505,8 +517,11 @@
/// Advances the reader by the specified number of bytes.
/// </summary>
/// <param name="count">The number of bytes to advance the reader.</param>
/// <exception cref="ObjectDisposedException">The writer has been disposed.</exception>
public void AdvanceReader(int count)
{
ThrowIfDisposed();

#if NET6_0_OR_GREATER
ArgumentOutOfRangeException.ThrowIfNegative(count);
ArgumentOutOfRangeException.ThrowIfGreaterThan(count, UnconsumedLength);
Expand Down Expand Up @@ -908,7 +923,7 @@
/// Provides reader access to an <see cref="ArcBufferWriter"/>.
/// </summary>
/// <param name="writer">The writer.</param>
public readonly struct ArcBufferReader(ArcBufferWriter writer)

Check warning on line 926 in src/Orleans.Serialization/Buffers/ArcBufferWriter.cs

View workflow job for this annotation

GitHub Actions / Roslyn analyzers (net10.0, stable) 

ArcBufferReader should override the equality (==) and inequality (!=) operators (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1815)

Check warning on line 926 in src/Orleans.Serialization/Buffers/ArcBufferWriter.cs

View workflow job for this annotation

GitHub Actions / Roslyn analyzers (net10.0, stable) 

ArcBufferReader should override Equals (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1815)
{
/// <summary>
/// Gets the number of unconsumed bytes.
Expand Down Expand Up @@ -1025,36 +1040,96 @@
}

/// <summary>
/// Represents a slice of a <see cref="ArcBufferWriter"/>.
/// Represents a reference-counted slice of raw bytes from an <see cref="ArcBufferWriter"/>.
/// </summary>
/// <remarks>
/// Initializes a new instance of the <see cref="ArcBuffer"/> type.
/// An owned slice must be disposed when no longer needed. Copying this struct creates a borrowed view
/// whose lifetime is bounded by the owner. Use <see cref="Slice(int)"/> to obtain an independent owner.
/// Referenced bytes must remain unchanged while a slice is in use.
/// The default value is a valid, owner-free empty buffer.
/// </remarks>
/// <param name="first">The first page in the sequence.</param>
/// <param name="token">The token of the first page in the sequence.</param>
/// <param name="offset">The offset into the buffer at which this slice begins.</param>
/// <param name="length">The length of this slice.</param>
public struct ArcBuffer(ArcBufferPage first, int token, int offset, int length) : IDisposable
public struct ArcBuffer : IDisposable
{
/// <summary>
/// Creates a borrowed view over a valid, pinned page chain.
/// </summary>
/// <param name="first">The first page, or <see langword="null"/> for an owner-free empty buffer.</param>
/// <param name="token">The current token of <paramref name="first"/>.</param>
/// <param name="offset">The offset within <paramref name="first"/> at which the view begins.</param>
/// <param name="length">The number of bytes covered by the view.</param>
/// <remarks>
/// The owning buffer must keep the referenced pages pinned and their bytes unchanged while this view is in use.
/// Call <see cref="Pin"/> to acquire independent ownership.
/// </remarks>
/// <exception cref="ArgumentOutOfRangeException">
/// The offset is outside the first page, the length is negative or exceeds the linked pages,
/// or an owner-free buffer has a nonzero offset or length.
/// </exception>
/// <exception cref="InvalidOperationException">The page token or reference count is invalid.</exception>
public ArcBuffer(ArcBufferPage? first, int token, int offset, int length)
{
if (length < 0) throw new ArgumentOutOfRangeException(nameof(length));
if (offset < 0 || offset > (first?.Length ?? 0)) throw new ArgumentOutOfRangeException(nameof(offset));
if (first is null)
{
if (length != 0) throw new ArgumentOutOfRangeException(nameof(length));
}
else
{
first.CheckValidity(token);
var remaining = length - (first.Length - offset);
var page = first;
while (remaining > 0)
{
page = page.Next;
if (page is null) throw new ArgumentOutOfRangeException(nameof(length), "The length exceeds the bytes in the linked pages.");
page.CheckValidity(page.Version);
remaining -= page.Length;
}
}

First = first;
_firstPageToken = token;
Offset = offset;
Length = length;
}

// Writer and slice operations establish the bounds and keep the referenced pages pinned.
internal ArcBuffer(ArcBufferPage first, int offset, int length)
{
Debug.Assert(offset >= 0 && offset <= first.Length);
Debug.Assert(length >= 0);
First = first;
_firstPageToken = first.Version;
Offset = offset;
Length = length;
}

/// <summary>
/// Gets the token of the first page pointed to by this slice.
/// </summary>
private int _firstPageToken = token;
private int _firstPageToken;

/// <summary>
/// Gets the first page.
/// Gets the first page, or <see langword="null"/> for an owner-free empty buffer.
/// </summary>
public readonly ArcBufferPage First = first;
public readonly ArcBufferPage? First;

/// <summary>
/// Gets the offset into the first page at which this slice begins.
/// </summary>
public readonly int Offset = offset;
public readonly int Offset;

/// <summary>
/// Gets the length of this sequence.
/// </summary>
public readonly int Length = length;
public readonly int Length;

/// <summary>Gets an empty buffer which owns no pages.</summary>
public static ArcBuffer Empty => default;

/// <summary>Gets whether this buffer contains no bytes.</summary>
public readonly bool IsEmpty => Length == 0;

/// <summary>Copies the contents of this writer to a span.</summary>
public readonly int CopyTo(Span<byte> output)
Expand Down Expand Up @@ -1184,9 +1259,19 @@
}

/// <summary>
/// Throws if the buffer it no longer valid.
/// Throws if the buffer is no longer valid.
/// </summary>
private readonly void CheckValidity() => First.CheckValidity(_firstPageToken);
internal readonly void CheckValidity()
{
if (First is not null)
{
First.CheckValidity(_firstPageToken);
}
else if (Length != 0 || Offset != 0)
{
throw new InvalidOperationException("An owner-free buffer must be empty.");
}
}

/// <summary>
/// Creates a pinned slice from the specified offset to the end of this buffer.
Expand Down Expand Up @@ -1232,33 +1317,18 @@
Debug.Assert(offset >= 0);
Debug.Assert(length >= 0);
Debug.Assert(length <= Length - offset);
ArcBuffer result;

// Navigate to the offset page & calculate the offset into the page.
if (Offset + offset < First.Length || length == 0)
if (First is null) return Empty;
var page = First;
var pageOffset = Offset;
while (offset > page.Length - pageOffset || (offset == page.Length - pageOffset && length > 0))
{
// The slice starts within this page.
result = new ArcBuffer(First, token: _firstPageToken, Offset + offset, length);
}
else
{
// The slice starts within a subsequent page.
// Account for the first page, then navigate to the page which the offset falls in.
offset -= First.Length - Offset;
var page = First.Next;
offset -= page.Length - pageOffset;
page = page.Next;
Debug.Assert(page is not null);

while (offset >= page.Length)
{
offset -= page.Length;
page = page.Next;
Debug.Assert(page is not null);
}

result = new ArcBuffer(page, token: page.Version, offset, length);
pageOffset = 0;
}

return result;
return new ArcBuffer(page, pageOffset + offset, length);
}

/// <summary>
Expand All @@ -1267,6 +1337,13 @@
public readonly void Pin()
{
CheckValidity();
if (First is null) return;
if (IsEmpty)
{
First.Pin(_firstPageToken);
return;
}

var pageEnumerator = Pages.GetEnumerator();
if (pageEnumerator.MoveNext())
{
Expand All @@ -1286,6 +1363,15 @@
/// </summary>
public void Unpin()
{
if (First is null) return;
CheckValidity();
if (IsEmpty)
{
First.Unpin(_firstPageToken);
_firstPageToken = -1;
return;
}

var pageEnumerator = Pages.GetEnumerator();
if (pageEnumerator.MoveNext())
{
Expand Down Expand Up @@ -1321,7 +1407,14 @@
/// Returns an enumerator which can be used to enumerate the pages referenced by this instance.
/// </summary>
/// <returns>An enumerator for the data contained in this instance.</returns>
internal readonly PageSegmentEnumerator PageSegments => new(this);
internal readonly PageSegmentEnumerator PageSegments
{
get
{
CheckValidity();
return new(this);
}
}

/// <summary>
/// Returns an enumerator which can be used to enumerate the span segments referenced by this instance.
Expand Down Expand Up @@ -1390,7 +1483,7 @@
private int _position;
private ArcBufferPage? _page = slice.Length > 0 ? slice.First : null;

internal readonly ArcBufferPage First => Slice.First;
internal readonly ArcBufferPage? First => Slice.First;
internal readonly int Offset => Slice.Offset;
internal readonly int Length => Slice.Length;

Expand Down Expand Up @@ -1419,6 +1512,7 @@
/// <returns><see langword="true"/> if the enumerator was successfully advanced to the next element; <see langword="false"/> if the enumerator has passed the end of the collection.</returns>
public bool MoveNext()
{
if (_position == 0 && (Length == 0 || _page == First)) Slice.CheckValidity();
Debug.Assert(_position <= Length);
if (_page is null || _position == Length)
{
Expand All @@ -1430,7 +1524,6 @@
if (_page == First)
{
Debug.Assert(_position == 0);
Slice.CheckValidity();
var offset = Offset;
var length = Math.Min(Length, _page.Length - offset);
_position += length;
Expand Down
3 changes: 2 additions & 1 deletion src/Orleans.Serialization/Buffers/Reader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ public static class Reader
/// <param name="input">The input.</param>
/// <param name="session">The session.</param>
/// <returns>A new <see cref="Reader{TInput}"/>.</returns>
/// <remarks>The input must remain valid and its referenced bytes unchanged for the lifetime of the reader.</remarks>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Reader<ArcBufferReaderInput> Create(ArcBuffer input, SerializerSession session) => new(new ArcBufferReaderInput(in input), session, 0);

Expand Down Expand Up @@ -614,7 +615,7 @@ public void ForkFrom(long position, out Reader<TInput> forked)
else if (IsArcBufferInput)
{
ref var input = ref Unsafe.As<TInput, ArcBufferReaderInput>(ref _input);
var newInput = input.ForkFrom(checked((int)position));
var newInput = input.ForkFrom(checked((int)(position - _sequenceOffset)));
forked = new Reader<TInput>(Unsafe.As<ArcBufferReaderInput, TInput>(ref newInput), Session, position);

if (forked.Position != position)
Expand Down
Loading
Loading