From 1e2e283c0c08ef2803bb905fc8d3e07901bbce50 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Wed, 3 Jun 2026 18:42:24 +0800
Subject: [PATCH] Add canary to RC

We still occasionally see release overflow errors. I think this is a UAF, since
I cannot find any mismatched acquire/releases. So this adds a canary value to
every RC and includes it in the crash dump.

If the canary value is different than what we set it at, then we have a UAF.
If the canary value is set to the poison we set on release, then we have a
double free.
If the canary value is unchanged, then it's inconclusive.
---
 src/lightpanda.zig | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/lightpanda.zig b/src/lightpanda.zig
index 83214b4c..fb1659d4 100644
--- a/src/lightpanda.zig
+++ b/src/lightpanda.zig
@@ -288,10 +288,24 @@ noinline fn assertionFailure(comptime ctx: []const u8, args: anytype) noreturn {
     @import("crash_handler.zig").crash(ctx, args, @returnAddress());
 }
 
+// Written into every RC at construction (rc_canary) and overwritten with
+// rc_poison on the final release. We only get crash reports (not logs) from
+// prod, so reading _canary in the "release overflow" assert tells us which kind
+// of bug it is:
+//   - rc_canary ("RCNT"): the struct still looks live -> a real refcount
+//     accounting bug, OR the memory was reused by a freshly-built RC (which
+//     re-stamps the canary, so this case can't be fully ruled out).
+//   - rc_poison ("DEADC0DE"): a stale finalizer fired again on an object we
+//     already released, before its memory was reused -> UAF.
+//   - anything else: the memory was freed and reused by non-RC data -> UAF.
+const rc_canary: u32 = 0x52434E54;
+const rc_poison: u32 = 0xDEADC0DE;
+
 // Reference counting helper
 pub fn RC(comptime T: type) type {
     return struct {
         _refs: std.atomic.Value(T) = .init(0),
+        _canary: u32 = rc_canary,
 
         pub fn init(refs: T) @This() {
             return .{ ._refs = .init(refs) };
@@ -303,8 +317,17 @@ pub fn RC(comptime T: type) type {
 
         pub fn release(self: *@This(), value: anytype, page: *Page) void {
             const prev = self._refs.fetchSub(1, .acq_rel);
-            assert(prev > 0, "release overflow", .{ .type = @typeName(@TypeOf(value)) });
+            assert(prev > 0, "release overflow", .{
+                .type = @typeName(@TypeOf(value)),
+                .canary = self._canary, // rc_canary=live/accounting, rc_poison=double-release, else=reuse
+                .refs = prev,
+                .ptr = @intFromPtr(value),
+            });
             if (prev == 1) {
+                // Mark dead before deinit frees this memory, so a stale
+                // weak-callback re-fire reads rc_poison instead of a
+                // misleadingly-intact canary.
+                self._canary = rc_poison;
                 value.deinit(page);
             }
         }