[master] 94337ba52 ws: Reserve the requested size

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Tue Aug 17 06:57:05 UTC 2021


commit 94337ba5222371dbe489340a4ed5c36c1ecb5d7f
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed Jul 7 14:46:10 2021 +0200

    ws: Reserve the requested size
    
    Instead of guaranteeing the pointer alignment of ws->r at the expense of
    sometimes reserving more than requested, we can instead drop the pointer
    alignment requirement since that does not refer to the beginning of the
    allocation.
    
    Instead, it is in WS_Release() that we guarantee that the ws->f remains
    aligned for the next allocation. And WS_Release() already did that so we
    might as well avoid the potential overcommit from WS_ReserveSize().
    
    This is going to be helpful for a workspace emulator, to make sure that
    the alignment overhead wouldn't allow off-by-little overflows to sneak
    past the address sanitizer. This very change was initially made for a
    workspace sanitizer.
    
    It is also a good occasion to make the relevant checks around the ws->r
    pointer in WS_Assert().
    
    This change is visible in r03131.vtc that now exhibits this behavior.
    
    Refs #3320

diff --git a/bin/varnishd/cache/cache_ws.c b/bin/varnishd/cache/cache_ws.c
index f8c491ebb..784e3b493 100644
--- a/bin/varnishd/cache/cache_ws.c
+++ b/bin/varnishd/cache/cache_ws.c
@@ -58,9 +58,8 @@ WS_Assert(const struct ws *ws)
 	assert(ws->f <= ws->e);
 	assert(PAOK(ws->f));
 	if (ws->r) {
-		assert(ws->r > ws->s);
+		assert(ws->r >= ws->f);
 		assert(ws->r <= ws->e);
-		assert(PAOK(ws->r));
 	}
 	assert(*ws->e == WS_REDZONE_END);
 }
@@ -284,25 +283,22 @@ WS_ReserveAll(struct ws *ws)
 unsigned
 WS_ReserveSize(struct ws *ws, unsigned bytes)
 {
-	unsigned b2;
+	unsigned l;
 
 	WS_Assert(ws);
 	assert(ws->r == NULL);
 	assert(bytes > 0);
 
-	b2 = PRNDDN(ws->e - ws->f);
-	if (bytes < b2)
-		b2 = PRNDUP(bytes);
-
-	if (bytes > b2) {
+	l = pdiff(ws->f, ws->e);
+	if (bytes > l) {
 		WS_MarkOverflow(ws);
 		return (0);
 	}
-	ws->r = ws->f + b2;
-	DSL(DBG_WORKSPACE, 0, "WS_ReserveSize(%p, %u/%u) = %zu",
-	    ws, b2, bytes, pdiff(ws->f, ws->r));
+	ws->r = ws->f + bytes;
+	DSL(DBG_WORKSPACE, 0, "WS_ReserveSize(%p, %u/%u) = %u",
+	    ws, bytes, l, bytes);
 	WS_Assert(ws);
-	return (pdiff(ws->f, ws->r));
+	return (bytes);
 }
 
 unsigned
@@ -315,12 +311,11 @@ void
 WS_Release(struct ws *ws, unsigned bytes)
 {
 	WS_Assert(ws);
-	bytes = PRNDUP(bytes);
 	assert(bytes <= ws->e - ws->f);
 	DSL(DBG_WORKSPACE, 0, "WS_Release(%p, %u)", ws, bytes);
 	assert(ws->r != NULL);
 	assert(ws->f + bytes <= ws->r);
-	ws->f += bytes;
+	ws->f += PRNDUP(bytes);
 	ws->r = NULL;
 	WS_Assert(ws);
 }
diff --git a/bin/varnishtest/tests/r03131.vtc b/bin/varnishtest/tests/r03131.vtc
index ad6fc0e95..309e03b82 100644
--- a/bin/varnishtest/tests/r03131.vtc
+++ b/bin/varnishtest/tests/r03131.vtc
@@ -22,7 +22,7 @@ varnish v1 -vcl {
 
 logexpect l1 -v v1 -g raw {
 	expect * * VCL_Log	{^\Qres(8) = 8\E$}
-	expect 0 = VCL_Log	{^\Qres(15) = 16\E$}
+	expect 0 = VCL_Log	{^\Qres(15) = 15\E$}
 	expect 0 = VCL_Log	{^\Qres(16) = 16\E$}
 
 	expect 0 = VCL_Log	{^\Qres(17) = 0\E$}


More information about the varnish-commit mailing list