[master] 66e2ea313 http1: Missing workspace release

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Aug 30 05:43:07 UTC 2021


commit 66e2ea313e7d75530d90d5bd0df78f9609169c6e
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Aug 30 07:38:00 2021 +0200

    http1: Missing workspace release
    
    Working on the workspace sanitizer (ancestor of the workspace emulator)
    final rollbacks were needed to unwind allocations. There was however a
    branch where error handling was missing a workspace release, and it was
    fine before the introduction of the final rollbacks.
    
    To avoid turning the workspace emulator into a DoS vector the rollbacks
    are now only enforced for emulator builds. The specific "insufficient
    workspace" log is amended to ensure future changes to the session
    workspace footprint don't accidentally remove test coverage for that
    branch. The same could be done for other "insufficient workspace" logs
    in the PROXY protocol parsing.
    
    Refs 0632b84693f3 (req: Prevent early rollback)
    Refs ce71896ae353 (sess: Plug conceptual leak)
    Refs 246b1eb1e888 (busyobj: Plug conceptual leak)
    Refs 5b4f0f1addaa (htc: Defer workspace rollbacks for request tasks)
    Refs #3644
    
    Spotted by Alf's single process fuzzing setup that we should eventually
    revisit.
    
    Refs #3152

diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index 139112854..f3e41716f 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -181,7 +181,9 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo)
 	}
 
 	VCL_Rel(&bo->vcl);
+#ifdef ENABLE_WORKSPACE_EMULATOR
 	WS_Rollback(bo->ws, 0);
+#endif
 
 	memset(&bo->retries, 0,
 	    sizeof *bo - offsetof(struct busyobj, retries));
diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
index cff1d0470..09addfde4 100644
--- a/bin/varnishd/cache/cache_req.c
+++ b/bin/varnishd/cache/cache_req.c
@@ -172,7 +172,9 @@ Req_Release(struct req *req)
 	AZ(req->vcl);
 	if (req->vsl->wid)
 		VSL_End(req->vsl);
+#ifdef ENABLE_WORKSPACE_EMULATOR
 	WS_Rollback(req->ws, 0);
+#endif
 	TAKE_OBJ_NOTNULL(sp, &req->sp, SESS_MAGIC);
 	pp = sp->pool;
 	CHECK_OBJ_NOTNULL(pp, POOL_MAGIC);
diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index f3bb4da15..f3a9619c0 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -646,7 +646,9 @@ SES_Rel(struct sess *sp)
 	if (i)
 		return;
 	Lck_Delete(&sp->mtx);
+#ifdef ENABLE_WORKSPACE_EMULATOR
 	WS_Rollback(sp->ws, 0);
+#endif
 	MPL_Free(sp->pool->mpl_sess, sp);
 }
 
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index 393088520..da8c5fea6 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -119,7 +119,9 @@ http1_new_session(struct worker *wrk, void *arg)
 		/* Out of session workspace. Free the req, close the sess,
 		 * and do not set a new task func, which will exit the
 		 * worker thread. */
-		VSL(SLT_Error, req->sp->vxid, "insufficient workspace");
+		VSL(SLT_Error, req->sp->vxid,
+		    "insufficient workspace (proto_priv)");
+		WS_Release(req->ws, 0);
 		Req_Release(req);
 		SES_Delete(sp, SC_RX_JUNK, NAN);
 		return;
diff --git a/bin/varnishtest/tests/o00006.vtc b/bin/varnishtest/tests/o00006.vtc
new file mode 100644
index 000000000..5a6b4e357
--- /dev/null
+++ b/bin/varnishtest/tests/o00006.vtc
@@ -0,0 +1,46 @@
+varnishtest "SES_Reserve_proto_priv() overflow"
+
+feature ipv4
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -arg "-p pool_sess=0,0,0" -proto "PROXY" -vcl+backend {} -start
+
+logexpect l1 -v v1 -g raw {
+	expect 0 1000	Begin		"sess 0 PROXY"
+	expect 0 =	SessOpen
+	expect 0 =	Proxy		"2 217.70.181.33 60822 95.142.168.34 443"
+	expect 0 =	Error		{\Qinsufficient workspace (proto_priv)\E}
+	expect 0 =	SessClose	"RX_JUNK"
+} -start
+
+varnish v1 -cliok "param.set workspace_session 480"
+
+client c1 {
+	# PROXY2 with CRC32C TLV
+	sendhex {
+		0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a
+		21 11 00 65
+		d9 46 b5 21
+		5f 8e a8 22
+		ed 96
+		01 bb
+		03 00 04  95 03 ee 75
+		01 00 02  68 32
+		02 00 0a  68 6f 63 64 65 74 2e 6e 65 74
+		20 00 3d
+		01 00 00 00 00
+		21 00 07  54 4c 53 76 31 2e 33
+		25 00 05  45 43 32 35 36
+		24 00 0a  52 53 41 2d 53 48 41 32 35 36
+		23 00 16  41 45 41 44 2d 41 45 53 31 32 38
+			  2d 47 43 4d 2d 53 48 41 32 35 36
+	}
+	txreq
+	expect_close
+} -run
+
+logexpect l1 -wait


More information about the varnish-commit mailing list