[4.1] b06e54c Fix workspace overflow handling in VFP_Push() and test for it

Lasse Karstensen lkarsten at varnish-software.com
Wed Feb 17 14:30:54 CET 2016


commit b06e54c6d188f3b96cbe934e38e0a3918548e871
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Wed Feb 17 10:28:38 2016 +0100

    Fix workspace overflow handling in VFP_Push() and test for it
    
    This fix avoids a WS_Alloc panic when pushing fetch processors
    and should thus avoid panics by overflowing the backend workspace in vcl.
    
    This overflow is logged as FetchError "Bo workspace overflowed".
    
    Other panic points due to workspace_backend being set too low still exist.
    
    Sizing estimate for workspace_backend:
    
    	sizeof(struct busyobj)
    	+ 3 * HTTP_estimate(cache_param->http_max_hdr)
    	+ cache_param->vsl_buffer
    	+ cache_param->http_resp_size
    	+ fetch processor memory
    	+ space required in VCL
    
    Fixes #1739

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 3d0517c..c41fce3 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -534,6 +534,15 @@ vbf_fetch_body_helper(struct busyobj *bo)
 /*--------------------------------------------------------------------
  */
 
+#define vbf_vfp_push(bo, vfp, top)					\
+	if (VFP_Push((bo)->vfc, (vfp), (top)) == NULL) {		\
+		assert (WS_Overflowed((bo)->vfc->http->ws));		\
+		(void)VFP_Error((bo)->vfc, "Bo workspace overflowed");	\
+		(bo)->htc->doclose = SC_OVERLOAD;			\
+		VDI_Finish((bo)->wrk, bo);				\
+		return (F_STP_ERROR);					\
+	}
+
 static enum fetch_step
 vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 {
@@ -589,19 +598,19 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	assert(bo->do_gzip == 0 || bo->do_gunzip == 0);
 
 	if (bo->do_gunzip || (bo->is_gzip && bo->do_esi))
-		(void)VFP_Push(bo->vfc, &vfp_gunzip, 1);
+		vbf_vfp_push(bo, &vfp_gunzip, 1);
 
 	if (bo->htc->content_length != 0) {
 		if (bo->do_esi && bo->do_gzip) {
-			(void)VFP_Push(bo->vfc, &vfp_esi_gzip, 1);
+			vbf_vfp_push(bo, &vfp_esi_gzip, 1);
 		} else if (bo->do_esi && bo->is_gzip && !bo->do_gunzip) {
-			(void)VFP_Push(bo->vfc, &vfp_esi_gzip, 1);
+			vbf_vfp_push(bo, &vfp_esi_gzip, 1);
 		} else if (bo->do_esi) {
-			(void)VFP_Push(bo->vfc, &vfp_esi, 1);
+			vbf_vfp_push(bo, &vfp_esi, 1);
 		} else if (bo->do_gzip) {
-			(void)VFP_Push(bo->vfc, &vfp_gzip, 1);
+			vbf_vfp_push(bo, &vfp_gzip, 1);
 		} else if (bo->is_gzip && !bo->do_gunzip) {
-			(void)VFP_Push(bo->vfc, &vfp_testgunzip, 1);
+			vbf_vfp_push(bo, &vfp_testgunzip, 1);
 		}
 	}
 
diff --git a/bin/varnishd/cache/cache_fetch_proc.c b/bin/varnishd/cache/cache_fetch_proc.c
index 5333c62..6c38122 100644
--- a/bin/varnishd/cache/cache_fetch_proc.c
+++ b/bin/varnishd/cache/cache_fetch_proc.c
@@ -196,8 +196,11 @@ VFP_Push(struct vfp_ctx *vc, const struct vfp *vfp, int top)
 
 	CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(vc->http, HTTP_MAGIC);
+
 	vfe = WS_Alloc(vc->http->ws, sizeof *vfe);
-	AN(vfe);
+	if (vfe == NULL)
+		return NULL;
+
 	INIT_OBJ(vfe, VFP_ENTRY_MAGIC);
 	vfe->vfp = vfp;
 	vfe->closed = VFP_OK;
diff --git a/bin/varnishtest/tests/r01739.vtc b/bin/varnishtest/tests/r01739.vtc
new file mode 100644
index 0000000..5b80f1e
--- /dev/null
+++ b/bin/varnishtest/tests/r01739.vtc
@@ -0,0 +1,29 @@
+varnishtest "Check workspace overflow in fetch processor"
+
+server s1 {
+	rxreq
+	txresp -bodylen 1024
+} -start
+
+
+varnish v1 \
+ -vcl+backend {
+	import ${vmod_debug};
+	sub vcl_backend_response {
+		set beresp.do_gzip = true;
+		debug.workspace_allocate(backend, debug.workspace_free(backend) - 16);
+	}
+} -start
+
+logexpect l1 -v v1 -g raw {
+	expect * 1002 FetchError {^Bo workspace overflowed}
+	expect * =    Error      {^out of workspace [(]Bo[)]}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 503
+} -run
+
+logexpect l1 -wait



More information about the varnish-commit mailing list