[master] 8656fec WS: Fix off-by-one in WS_Reset

Tollef Fog Heen tfheen at err.no
Tue Jan 26 09:57:55 CET 2016


commit 8656fec9848ce71e995f0abadb822c91d58a1c07
Author: Devon H. O'Dell <dho at fastly.com>
Date:   Thu Jan 14 15:20:22 2016 -0800

    WS: Fix off-by-one in WS_Reset
    
    WS_Assert allows the workspace to be full in its check that `ws->f <= ws->e`.
    WS_Reset does not honor this; if a snapshot was taken of a full workspace, and
    that snapshot is reverted with WS_Reset, we panic.
    
    This issue was identified by Jozef Hatala <jozef at fastly.com>

diff --git a/bin/varnishd/cache/cache_ws.c b/bin/varnishd/cache/cache_ws.c
index a1b81eb..9293d9e 100644
--- a/bin/varnishd/cache/cache_ws.c
+++ b/bin/varnishd/cache/cache_ws.c
@@ -116,7 +116,7 @@ WS_Reset(struct ws *ws, char *p)
 		ws->f = ws->s;
 	else {
 		assert(p >= ws->s);
-		assert(p < ws->e);
+		assert(p <= ws->e);
 		ws->f = p;
 	}
 	ws_ClearOverflow(ws);
diff --git a/bin/varnishtest/tests/c00074.vtc b/bin/varnishtest/tests/c00074.vtc
new file mode 100644
index 0000000..f20f71c
--- /dev/null
+++ b/bin/varnishtest/tests/c00074.vtc
@@ -0,0 +1,25 @@
+varnishtest "Test WS_Reset off-by-one when workspace is full"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend {
+	import ${vmod_debug};
+	import ${vmod_std};
+
+	sub vcl_recv {
+		set req.http.ws-free = debug.workspace_free(session);
+		debug.workspace_allocate(session, std.integer(req.http.ws-free, 0));
+		debug.workspace_snap(session);
+		debug.workspace_reset(session);
+	}
+} -start
+
+client c1 {
+	txreq -url /foo
+	rxresp
+	expect resp.status == 200
+} -run
+
diff --git a/lib/libvmod_debug/vmod.vcc b/lib/libvmod_debug/vmod.vcc
index 9218b5e..8bc0dd2 100644
--- a/lib/libvmod_debug/vmod.vcc
+++ b/lib/libvmod_debug/vmod.vcc
@@ -139,6 +139,14 @@ $Function INT workspace_free(ENUM { client, backend, session, thread })
 
 Find how much unallocated space there is left in a workspace.
 
+$Function VOID workspace_snap(ENUM { client, backend, session, thread})
+
+Snapshot the named workspace. Only one snapshot may be active at a time.
+
+$Function VOID workspace_reset(ENUM { client, backend, session, thread })
+
+Reset to the previous snapshot of a workspace, taken from debug.workspace_snap.
+
 $Function VOID vcl_release_delay(DURATION)
 
 Hold a reference to the VCL when it goes cold for the given delay.
diff --git a/lib/libvmod_debug/vmod_debug.c b/lib/libvmod_debug/vmod_debug.c
index b87e673..839d0ad 100644
--- a/lib/libvmod_debug/vmod_debug.c
+++ b/lib/libvmod_debug/vmod_debug.c
@@ -419,6 +419,31 @@ vmod_workspace_overflowed(VRT_CTX, VCL_ENUM which)
 	return (WS_Overflowed(ws));
 }
 
+static char *debug_ws_snap;
+void
+vmod_workspace_snap(VRT_CTX, VCL_ENUM which)
+{
+	struct ws *ws;
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	ws = wsfind(ctx, which);
+	WS_Assert(ws);
+
+	debug_ws_snap = WS_Snapshot(ws);
+}
+
+void
+vmod_workspace_reset(VRT_CTX, VCL_ENUM which)
+{
+	struct ws *ws;
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	ws = wsfind(ctx, which);
+	WS_Assert(ws);
+
+	WS_Reset(ws, debug_ws_snap);
+}
+
 void
 vmod_workspace_overflow(VRT_CTX, VCL_ENUM which)
 {



More information about the varnish-commit mailing list