From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: mbuf cluster m_extref_mtx contention
To: David Gwynne <david@gwynne.id.au>, Christian Ludwig <cludwig@genua.de>
Cc: tech@openbsd.org
Date: Thu, 11 Jun 2026 13:44:26 +0200

On Fri, Apr 24, 2026 at 04:43:03PM +0200, Alexander Bluhm wrote:
> On Wed, Apr 22, 2026 at 04:21:42PM +1000, David Gwynne wrote:
> > from another perspective, if the use after free is the result
> > mishandling mbufs, then the other diff is more forgiving of a buggy
> > behaviour.
> 
> I still see stability problems with the extfree proxy diff.  It
> only happens when doing creating kstack flamegraphs with btrace.

cludwig@ gave me a hint where r_traceidx in refcnt_rele() is
problematic.

thread A calls atomic_dec_int_nv(&r->r_refs), count from 2 to 1
thread B calls atomic_dec_int_nv(&r->r_refs), set refs to 0
thread B reads r->r_traceidx
thread B frees and poisons struct refcnt
thread A reads r->r_traceidx from invalid memory

This scenario is unlikely as thread B has to do a lot of work between
consecutive instructions on thread A.  But it is wrong anyway.

With the diff below, I tested dlg@'s mbuf-ext-proxy diff multiple
times without crash.

ok for the refcnt_rele() trace index fix?

bluhm

Index: kern/kern_synch.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_synch.c,v
diff -u -p -r1.233 kern_synch.c
--- kern/kern_synch.c	24 Nov 2025 12:54:53 -0000	1.233
+++ kern/kern_synch.c	11 Jun 2026 11:35:50 -0000
@@ -926,11 +926,13 @@ int
 refcnt_rele(struct refcnt *r)
 {
 	u_int refs;
+	int trace;
 
+	trace = r->r_traceidx;
 	membar_exit_before_atomic();
 	refs = atomic_dec_int_nv(&r->r_refs);
 	KASSERT(refs != ~0);
-	TRACEINDEX(refcnt, r->r_traceidx, r, refs + 1, -1);
+	TRACEINDEX(refcnt, trace, r, refs + 1, -1);
 	if (refs == 0) {
 		membar_enter_after_atomic();
 		return (1);
@@ -949,17 +951,19 @@ void
 refcnt_finalize(struct refcnt *r, const char *wmesg)
 {
 	u_int refs;
+	int trace;
 
+	trace = r->r_traceidx;
 	membar_exit_before_atomic();
 	refs = atomic_dec_int_nv(&r->r_refs);
 	KASSERT(refs != ~0);
-	TRACEINDEX(refcnt, r->r_traceidx, r, refs + 1, -1);
+	TRACEINDEX(refcnt, trace, r, refs + 1, -1);
 	while (refs) {
 		sleep_setup(r, PWAIT, wmesg);
 		refs = atomic_load_int(&r->r_refs);
 		sleep_finish(INFSLP, refs);
 	}
-	TRACEINDEX(refcnt, r->r_traceidx, r, refs, 0);
+	TRACEINDEX(refcnt, trace, r, refs, 0);
 	/* Order subsequent loads and stores after refs == 0 load. */
 	membar_sync();
 }