[master] 356854c Fix workspace overflow handling in VFP_Push() and test for it

Nils Goroll nils.goroll at uplex.de
Wed Feb 17 11:23:09 CET 2016


commit 356854cd86b1866b492bbe64417d1ace6e19a9b3
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 c168108..c210bd3 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -537,6 +537,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)
 {
@@ -592,19 +601,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 cb0e1d1..ce9d138 100644
--- a/bin/varnishd/cache/cache_fetch_proc.c
+++ b/bin/varnishd/cache/cache_fetch_proc.c
@@ -203,8 +203,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