Hi,

I was recently debugging a protection fault in a kernel based on OpenBSD 7.7.
Iterating over a SRP list with SRPL_FOREACH seemingly returned a corrupted
pointer. I did trace the issue down to a bug in the SRP implementation.

srp_follow() is supposed to replace the reference held by the given srp_ref with
a reference to the data structure that is represented by the given srp struct.
This process is not atomic though. The old reference is given up immediately in
srp_v() when one part of the hazard is updated with 'hzrd->sh_p = srp'.
Afterwards the srp struct is read to get the value represented by that struct.

This causes a problem when srp_follow() is used to iterate over SRP lists. The
current list element is only guaranteed to be kept alive while the hazard inside
the srp_ref still points to it. As soon as that reference is given up in the
call to srp_follow() the element may be freed. With SRP lists the srp struct
that is read afterwards lives inside of the current element though. Accessing it
to get a reference to the next list element can therefore be a use-after-free.

The fix is simple: Use srp_enter() to get a new reference to the next element
while keeping the current element alive. Afterwards the old reference can safely
be released and the hazard in the caller provided srp_ref struct can be updated
to the hazard of the new element.

diff --git sys/kern/kern_srp.c sys/kern/kern_srp.c
index c002fa391fd..7b702b9bde2 100644
--- sys/kern/kern_srp.c
+++ sys/kern/kern_srp.c
@@ -235,7 +235,14 @@ srp_enter(struct srp_ref *sr, struct srp *srp)
 void *
 srp_follow(struct srp_ref *sr, struct srp *srp)
 {
-	return (srp_v(sr->hz, srp));
+	struct srp_ref nsr;
+	void *nv;
+
+	nv = srp_enter(&nsr, srp);
+	srp_leave(sr);
+	sr->hz = nsr.hz;
+
+	return (nv);
 }
 
 void

• smime.p7s (5K)