Skip to content

Commit 243c439

Browse files
committed
feat(pool): pooled-buffer view-in-a-field dangle — Memory view of an IMemoryOwner field read after Dispose → OWN002
Generalises the field-mediated use-after-dispose pass (#75/#76) to POOLED owners and their Memory views. An `IMemoryOwner<T>` field released in `Dispose()` is a released owner, and a `Memory`/`ReadOnlyMemory` field assigned `_owner.Memory` (recognised by ViewOwner, which returns the owner name for a bare-field receiver) is recorded as a VIEW of that owner. A read of the view field — or of the owner directly — in a live subscription-target handler (directly or one hop through a private helper), with no `if (_disposed) return;` guard before it, is lowered to a synthetic acquire/release/use flow on the owner → OWN002. The view-field aliases the owner, so reading it after the owner's release is a use of the owner after release. New `ReleasedOwner(field)` unifies the two: the field itself when it is a released owner, or the owner it aliases when it is a view field. `FirstUnguardedDisposedRead` and the handler trigger loop both route through it, so direct disposed-field reads (#75/#76) are unchanged and view-field reads are additive. Precision: same handler reachability that makes the disposed-field UAF sound (a live handler can fire after Dispose). The owner must be released in the dispose lifecycle; the view→owner alias is symbol-resolved (`IMemoryOwner<T>.Memory`); a guarded/unsubscribed handler never fires. Exposing such a view via a public getter is deliberately NOT flagged — `IMemoryOwner.Memory` is itself a valid "valid-until-Dispose" pattern, so only a provably-post-release read is a bug. New corpus case corpus/wpf/pooled-view-after-dispose (before → OWN002, guarded after → silent). Benchmark recall floor 22 -> 23. Scope: IMemoryOwner owner + member-access view read; ArrayPool byte[]-buffer-field views, bare-arg reads, and element-access reads are honest follow-ups. Python suite + metamorphic green; C# validated by CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent 5b31c36 commit 243c439

7 files changed

Lines changed: 206 additions & 17 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -803,8 +803,10 @@ jobs:
803803
# field disposed in `Dispose()` and read in a live subscription-target handler (RHS of a `+=` / arg
804804
# of a `.Subscribe(...)`, not torn down, no `if (_disposed) return;` guard) — DIRECTLY
805805
# (`field-use-after-dispose`) or ONE hop down through a private helper (`handler-use-after-dispose`)
806-
# — lowered to a synthetic acquire/release/use flow -> OWN002. Remaining backlog: a view stored in a
807-
# FIELD, a TWO-plus-hop indirect field use, and an injected-source region-escape. A drop below the
808-
# floor is a regression.
809-
run: python scripts/benchmark.py --min-recall 22
806+
# — lowered to a synthetic acquire/release/use flow -> OWN002. That pass also covers POOLED owners:
807+
# an `IMemoryOwner<T>` field released in `Dispose()` and a `Memory` VIEW field of it (`_view =
808+
# _owner.Memory`) read in such a handler is the view-in-a-field dangle -> OWN002 (`pooled-view-after-
809+
# dispose`). Remaining backlog: an ArrayPool `byte[]`-buffer-field view, a TWO-plus-hop indirect field
810+
# use, and an injected-source region-escape. A drop below the floor is a regression.
811+
run: python scripts/benchmark.py --min-recall 23
810812

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// FIXED. The callback guards on the disposed flag before it touches the view, so a late, already-
2+
// dispatched callback bails before reading a buffer that has been returned to the pool. (Equivalently,
3+
// drain the dispatcher queue before disposing.) The extractor sees the opening `if (_disposed) return;`
4+
// guard precede the view read and stays silent.
5+
using System;
6+
using System.Buffers;
7+
8+
public sealed class FrameDecoder : IDisposable
9+
{
10+
private readonly IMemoryOwner<byte> _owner;
11+
private readonly Memory<byte> _view;
12+
private readonly IDisposable _sub;
13+
private bool _disposed;
14+
15+
public FrameDecoder(int n, IEventBus bus)
16+
{
17+
_owner = MemoryPool<byte>.Shared.Rent(n);
18+
_view = _owner.Memory;
19+
_sub = bus.Subscribe<FrameReady>(OnFrameReady);
20+
}
21+
22+
private void OnFrameReady(FrameReady e)
23+
{
24+
if (_disposed) return; // do not touch the pooled buffer after it is returned
25+
Sink.Write(_view.Span);
26+
}
27+
28+
public void Dispose()
29+
{
30+
_disposed = true;
31+
_sub.Dispose();
32+
_owner.Dispose();
33+
}
34+
}
35+
36+
// Minimal in-file stand-ins so the reduction is self-contained.
37+
public interface IEventBus
38+
{
39+
IDisposable Subscribe<T>(Action<T> handler);
40+
}
41+
42+
public sealed class FrameReady { }
43+
44+
internal static class Sink
45+
{
46+
public static void Write(ReadOnlySpan<byte> data) { }
47+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// BUGGY (representative pooled-buffer pattern; the C# extractor catches this end-to-end via its
2+
// field-mediated use-after-dispose pass extended to pooled owners and their Memory views).
3+
//
4+
// A type rents a pooled buffer (`IMemoryOwner<byte>` from `MemoryPool<byte>`), keeps a `Memory<byte>`
5+
// VIEW of it IN A FIELD, and subscribes a handler to an event source. On teardown Dispose() returns
6+
// the buffer to the pool (disposing the owner), but a callback already queued on the dispatcher still
7+
// runs after Dispose() and reads the field-held view — a `Memory` backed by a buffer already handed
8+
// back to the pool: a dangling borrow / use-after-free (an `ObjectDisposedException` or stale/torn
9+
// bytes from the next renter). The view field aliases the owner, so reading it after the owner's
10+
// Dispose is a use of the owner after release. The fix (after.cs) guards the handler on a disposed
11+
// flag before it touches the view.
12+
using System;
13+
using System.Buffers;
14+
15+
public sealed class FrameDecoder : IDisposable
16+
{
17+
private readonly IMemoryOwner<byte> _owner;
18+
private readonly Memory<byte> _view;
19+
private readonly IDisposable _sub;
20+
21+
public FrameDecoder(int n, IEventBus bus)
22+
{
23+
_owner = MemoryPool<byte>.Shared.Rent(n);
24+
_view = _owner.Memory; // a view of the pooled buffer, kept in a field
25+
_sub = bus.Subscribe<FrameReady>(OnFrameReady);
26+
}
27+
28+
private void OnFrameReady(FrameReady e)
29+
{
30+
// a late, already-dispatched callback: runs after Dispose()
31+
Sink.Write(_view.Span); // <-- reads a view of a buffer already returned to the pool
32+
}
33+
34+
public void Dispose()
35+
{
36+
_sub.Dispose();
37+
_owner.Dispose(); // pooled buffer returned; _view now dangles
38+
}
39+
}
40+
41+
// Minimal in-file stand-ins so the reduction is self-contained.
42+
public interface IEventBus
43+
{
44+
IDisposable Subscribe<T>(Action<T> handler);
45+
}
46+
47+
public sealed class FrameReady { }
48+
49+
internal static class Sink
50+
{
51+
public static void Write(ReadOnlySpan<byte> data) { }
52+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// OwnLang model of the pooled-buffer view-in-a-field dangle. `acquire` == MemoryPool.Rent (the
2+
// IMemoryOwner), `release` == its Dispose() in the type's Dispose() (the pooled buffer goes back to
3+
// the pool), `use` == a late dispatcher callback (the subscribed handler) reading the owner's Memory
4+
// VIEW — held in a field — AFTER Dispose. The view field aliases the owner, so reading it after the
5+
// owner's release is a use of the owner after release, lowered to OWN002. (The C# bridge tags the
6+
// synthetic flow with the generic `disposable` kind; here it is named `pooled buffer` to describe the
7+
// modelled resource.)
8+
module PooledViewAfterDispose
9+
10+
resource PooledBuffer {
11+
acquire Rent
12+
release Dispose
13+
kind "pooled buffer"
14+
}
15+
16+
fn OnFrameReady(bus: int) {
17+
let owner = acquire PooledBuffer(bus);
18+
release owner; // the type's Dispose() returns the pooled buffer
19+
use owner; // a late callback reads the owner's Memory view (a field) after Dispose -> OWN002
20+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
OWN002
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Pooled-buffer view-in-a-field, read after the owner is returned
2+
3+
**Pattern:** a type rents a pooled buffer (`IMemoryOwner<byte>` from `MemoryPool<byte>`), keeps a
4+
`Memory<byte>` **view of it in a field**, and subscribes a handler to an event source. On teardown
5+
`Dispose()` returns the buffer to the pool (disposing the owner), but a callback already queued on the
6+
dispatcher still runs after `Dispose()` and reads the field-held view — a `Memory` backed by a buffer
7+
already handed back to the pool: a dangling borrow / use-after-free (an `ObjectDisposedException`, or
8+
stale/torn bytes once the buffer is re-rented). It is the pooled-buffer, field-stored cousin of the
9+
local returned-view dangle (`memorypool-view-after-dispose`) and of the disposed-field UAF
10+
(`field-use-after-dispose`).
11+
12+
**What's new — the extractor follows the view-field to its owner.** The field-mediated
13+
use-after-dispose pass (#75/#76) is generalised to **pooled owners** and their **Memory views**: an
14+
`IMemoryOwner<T>` field released in `Dispose()` is a released owner, and a `Memory`/`ReadOnlyMemory`
15+
field assigned `_owner.Memory` (or `_buf.AsMemory(...)`) is recorded as a **view of that owner**. A
16+
read of the view field (or of the owner directly) in a live subscription-target handler — directly or
17+
one hop through a private helper — with no `if (_disposed) return;` guard before it, is lowered to a
18+
synthetic `acquire`/`release`/`use` flow on the owner → **OWN002**, via the existing OwnIR bridge (no
19+
new diagnostic). On the real C# the `corpus-benchmark` job scores `before.cs` as caught and the
20+
guarded `after.cs` as silent.
21+
22+
**Precision (why it stays low-FP).** The same reachability that makes the disposed-field UAF sound: a
23+
live (not unsubscribed) handler can fire *after* `Dispose()`, so reading the owner's buffer (through
24+
its view field) then is a use-after-release. The guard exclusion is the canonical fix; an unsubscribed
25+
or guarded handler never fires; the owner must be released in the dispose lifecycle; and the view→owner
26+
alias is recognised by the resolved `IMemoryOwner<T>.Memory` / `MemoryExtensions.AsMemory` symbols, not
27+
by name. Exposing such a view via a public getter is **not** flagged — `IMemoryOwner.Memory` is itself
28+
a legitimate "valid until Dispose" pattern, so the exposure is sound; only a provably-post-release read
29+
(the handler reachability) is a bug.
30+
31+
**Honesty / scope.** This slice covers an **`IMemoryOwner<T>`** owner field and its `Memory` view read
32+
via member access (`_view.Span`, `_view.Length`, …) in a handler/helper. Honest follow-ups: an
33+
**ArrayPool `byte[]` buffer field** returned in `Dispose()` (it interacts with the existing per-member
34+
pool-leak pass), the view passed as a **bare argument** (`Consume(_view)`) rather than through a member
35+
access, and **element-access** reads (`_buf[i]`). `case.own` is a faithful hand reduction of the
36+
ownership logic; `before.cs` / `after.cs` are representative of the bug and its fix.
37+
38+
Reference: [P-007](../../../docs/proposals/P-007-arraypool-span.md); the local twin is
39+
`memorypool-view-after-dispose`; the disposed-field twins are `field-use-after-dispose` /
40+
`handler-use-after-dispose`.

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -947,20 +947,31 @@ static bool DisposedGuardBefore(BlockSyntax body, int before) =>
947947
|| (ifs.Statement is BlockSyntax gb
948948
&& gb.Statements.FirstOrDefault() is ReturnStatementSyntax)));
949949

950-
// The FIRST direct read of a disposed field (`_f.Member`, `_f`/`this._f` only) in `body` that is
951-
// NOT protected by an opening disposed-guard — the unit the field-UAF pass keys on. Returns the
952-
// member-access node and the field name, or null when the body has no such read (or its first
953-
// disposed-field read is already guarded, the canonical-fix shape). Shared by the direct handler
954-
// scan and the one-hop helper scan (an indirect use through a private helper).
950+
// The released OWNER field reached by a read of field `f`: `f` itself when it is a released owner
951+
// (an IDisposable or `IMemoryOwner<T>` field released in Dispose), or — when `f` is a `Memory` VIEW
952+
// field — the owner it aliases, provided that owner is released. Null when `f` reaches no released
953+
// owner. Unifies the direct disposed-field read (#75/#76) with the pooled-view-in-a-field dangle:
954+
// reading `_view` (a borrow of `_owner`) after `_owner`'s release is a use of `_owner` after release.
955+
static string? ReleasedOwner(string f, Dictionary<string, int> releasedAt,
956+
Dictionary<string, string> viewFieldOwner) =>
957+
releasedAt.ContainsKey(f) ? f
958+
: viewFieldOwner.TryGetValue(f, out var owner) && releasedAt.ContainsKey(owner) ? owner
959+
: null;
960+
961+
// The FIRST read of a released owner — directly (`_f.Member`) or through a Memory VIEW field that
962+
// aliases it — in `body` that is NOT protected by an opening disposed-guard. Returns the member-
963+
// access node and the OWNER field, or null when the body has no such read (or its first such read is
964+
// already guarded, the canonical-fix shape). Shared by the direct handler scan and the one-hop
965+
// helper scan (an indirect use through a private helper).
955966
static (MemberAccessExpressionSyntax Use, string Field)? FirstUnguardedDisposedRead(
956-
BlockSyntax body, Dictionary<string, int> releasedAt)
967+
BlockSyntax body, Dictionary<string, int> releasedAt, Dictionary<string, string> viewFieldOwner)
957968
{
958969
foreach (var ma in body.DescendantNodes().OfType<MemberAccessExpressionSyntax>())
959970
{
960971
if (ma.Name.Identifier.Text is "Dispose" or "DisposeAsync")
961972
continue;
962-
if (ThisFieldName(ma.Expression) is { } uf && releasedAt.ContainsKey(uf))
963-
return DisposedGuardBefore(body, ma.SpanStart) ? null : (ma, uf);
973+
if (ThisFieldName(ma.Expression) is { } f && ReleasedOwner(f, releasedAt, viewFieldOwner) is { } owner)
974+
return DisposedGuardBefore(body, ma.SpanStart) ? null : (ma, owner);
964975
}
965976
return null;
966977
}
@@ -1972,13 +1983,16 @@ or ImplicitObjectCreationExpressionSyntax
19721983
// Gated on --flow-locals like the rest of the synthetic-flow emission.
19731984
if (flowLocals)
19741985
{
1975-
// IDisposable fields -> declaration line (the synthetic `acquire`).
1986+
// Owner fields -> declaration line (the synthetic `acquire`): IDisposable fields AND
1987+
// `IMemoryOwner<T>` MemoryPool rentals, whose pooled buffer is released by Dispose() — so a
1988+
// read after that Dispose (including through a Memory VIEW field of the owner) dangles.
19761989
var dispoFieldLine = new Dictionary<string, int>(StringComparer.Ordinal);
19771990
foreach (var fd in cls.Members.OfType<FieldDeclarationSyntax>())
19781991
{
19791992
if (fd.Modifiers.Any(mm => mm.IsKind(SyntaxKind.StaticKeyword)))
19801993
continue;
1981-
if (!IsDisposableType(fd.Declaration.Type.ToString()))
1994+
var ftype = fd.Declaration.Type.ToString();
1995+
if (!IsDisposableType(ftype) && !ftype.StartsWith("IMemoryOwner", StringComparison.Ordinal))
19821996
continue;
19831997
foreach (var v in fd.Declaration.Variables)
19841998
dispoFieldLine[v.Identifier.Text] = LineOf(v);
@@ -2002,6 +2016,18 @@ or ImplicitObjectCreationExpressionSyntax
20022016
}
20032017
if (releasedAt.Count > 0)
20042018
{
2019+
// a Memory/ReadOnlyMemory VIEW field that aliases an owner field — `_view = _owner.Memory`
2020+
// (an IMemoryOwner rental) or `_view = _buf.AsMemory(...)` — mapped to its OWNER, so a read
2021+
// of the VIEW field after the owner's release is a use of the released owner (the pooled-
2022+
// buffer view-in-a-field dangle). `ViewOwner` returns the owner name for a bare-field
2023+
// receiver; only owners actually released (in `releasedAt`) fire, checked at the read.
2024+
var viewFieldOwner = new Dictionary<string, string>(StringComparer.Ordinal);
2025+
foreach (var a in assigns)
2026+
if (a.IsKind(SyntaxKind.SimpleAssignmentExpression)
2027+
&& FieldName(a.Left) is { } vf
2028+
&& ViewOwner(a.Right, model) is { } vo)
2029+
viewFieldOwner[vf] = vo;
2030+
20052031
// handler method names that are LIVE subscription targets. `+=` subscriptions are
20062032
// keyed by SOURCE|handler so a `-=` removes only the MATCHING one — a handler still
20072033
// `+=`'d to another live source stays live (a name-only set would let one `-=` drop
@@ -2039,7 +2065,7 @@ or ImplicitObjectCreationExpressionSyntax
20392065
&& hm.Identifier.Text is not ("Dispose" or "DisposeAsync")
20402066
&& IsPrivateInstanceHelper(hm)
20412067
&& model.GetDeclaredSymbol(hm) is { } hsym
2042-
&& FirstUnguardedDisposedRead(b, releasedAt) is { } r)
2068+
&& FirstUnguardedDisposedRead(b, releasedAt, viewFieldOwner) is { } r)
20432069
helperReads[hsym] = (r.Field, LineOf(r.Use));
20442070

20452071
foreach (var hm in cls.Members.OfType<MethodDeclarationSyntax>())
@@ -2062,9 +2088,10 @@ or ImplicitObjectCreationExpressionSyntax
20622088
{
20632089
if (node is MemberAccessExpressionSyntax ma
20642090
&& ma.Name.Identifier.Text is not ("Dispose" or "DisposeAsync")
2065-
&& ThisFieldName(ma.Expression) is { } uf && releasedAt.ContainsKey(uf))
2091+
&& ThisFieldName(ma.Expression) is { } uf
2092+
&& ReleasedOwner(uf, releasedAt, viewFieldOwner) is { } owner)
20662093
{
2067-
useField = uf; useLine = LineOf(ma); triggerPos = ma.SpanStart;
2094+
useField = owner; useLine = LineOf(ma); triggerPos = ma.SpanStart;
20682095
break;
20692096
}
20702097
if (node is InvocationExpressionSyntax call

0 commit comments

Comments
 (0)