From: tedu <tedu@openbsd.org>
Date: Sat, 2 Jul 2016 17:09:09 +0000 (+0000)
Subject: check cache tree for collisions when inserting replies.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f6095d73eee33225fc0cbb4f30b159e4fbeb1f63;p=openbsd

check cache tree for collisions when inserting replies.
if two identical requests are sent out, the first will create a cache
entry. the second will not go into the cache tree, but will linger around,
causing a crash when we free it and try to remove from the tree. instead,
give up if insert fails.
diagnosis and initial patch from Duncan.
---

diff --git a/usr.sbin/rebound/rebound.c b/usr.sbin/rebound/rebound.c
index 60effd309b1..f398afba898 100644
--- a/usr.sbin/rebound/rebound.c
+++ b/usr.sbin/rebound/rebound.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: rebound.c,v 1.64 2016/06/05 22:41:41 tedu Exp $ */
+/* $OpenBSD: rebound.c,v 1.65 2016/07/02 17:09:09 tedu Exp $ */
 /*
  * Copyright (c) 2015 Ted Unangst <tedu@openbsd.org>
  *
@@ -313,16 +313,24 @@ sendreply(int ud, struct request *req)
 	resp->id = req->clientid;
 	sendto(ud, buf, r, 0, &req->from, req->fromlen);
 	if ((ent = req->cacheent)) {
+		/*
+		 * we do this first, because there's a potential race against
+		 * other requests made at the same time. if we lose, abort.
+		 * if anything else goes wrong, though, we need to reverse.
+		 */
+		if (RB_INSERT(cachetree, &cachetree, ent))
+			return;
 		ent->ts = now;
 		ent->ts.tv_sec += 10;
 		ent->resp = malloc(r);
-		if (!ent->resp)
+		if (!ent->resp) {
+			RB_REMOVE(cachetree, &cachetree, ent);
 			return;
+		}
 		memcpy(ent->resp, buf, r);
 		ent->resplen = r;
 		cachecount += 1;
 		TAILQ_INSERT_TAIL(&cachefifo, ent, fifo);
-		RB_INSERT(cachetree, &cachetree, ent);
 	}
 }