[master] 758b4bf4c ws: Move the workspace panic dump to cache_ws.c

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Aug 31 18:41:10 UTC 2020


commit 758b4bf4c9d0b2b7afc06b1e548a8592cdfd1ad4
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed May 6 22:30:35 2020 +0200

    ws: Move the workspace panic dump to cache_ws.c
    
    The goal is to avoid direct field access inside struct ws outside of
    cache_ws.c and open the possibility to perform a hexdump of a corrupted
    allocation in the future, when wssan panics while checking red zones.
    
    This effectively completes the ban of direct field access to the front,
    start and reservation pointers outside of cache_ws.c, and since the
    devil is in the details cache_http.c directly touches the id field and
    vmod_vtc also accesses the 3 aforementioned pointers.

diff --git a/bin/varnishd/cache/cache_panic.c b/bin/varnishd/cache/cache_panic.c
index 19ae14a00..fa1ff00d4 100644
--- a/bin/varnishd/cache/cache_panic.c
+++ b/bin/varnishd/cache/cache_panic.c
@@ -130,38 +130,6 @@ PAN_already(struct vsb *vsb, const void *ptr)
 
 /*--------------------------------------------------------------------*/
 
-static void
-pan_ws(struct vsb *vsb, const struct ws *ws)
-{
-
-	VSB_printf(vsb, "ws = %p {\n", ws);
-	if (PAN_already(vsb, ws))
-		return;
-	VSB_indent(vsb, 2);
-	PAN_CheckMagic(vsb, ws, WS_MAGIC);
-	if (ws->id[0] != '\0' && (!(ws->id[0] & 0x20)))
-		VSB_cat(vsb, "OVERFLOWED ");
-	VSB_printf(vsb, "id = \"%s\",\n", ws->id);
-	VSB_printf(vsb, "{s, f, r, e} = {%p", ws->s);
-	if (ws->f >= ws->s)
-		VSB_printf(vsb, ", +%ld", (long) (ws->f - ws->s));
-	else
-		VSB_printf(vsb, ", %p", ws->f);
-	if (ws->r >= ws->s)
-		VSB_printf(vsb, ", +%ld", (long) (ws->r - ws->s));
-	else
-		VSB_printf(vsb, ", %p", ws->r);
-	if (ws->e >= ws->s)
-		VSB_printf(vsb, ", +%ld", (long) (ws->e - ws->s));
-	else
-		VSB_printf(vsb, ", %p", ws->e);
-	VSB_cat(vsb, "},\n");
-	VSB_indent(vsb, -2);
-	VSB_cat(vsb, "},\n");
-}
-
-/*--------------------------------------------------------------------*/
-
 static void
 pan_htc(struct vsb *vsb, const struct http_conn *htc)
 {
@@ -174,7 +142,7 @@ pan_htc(struct vsb *vsb, const struct http_conn *htc)
 	if (htc->rfd != NULL)
 		VSB_printf(vsb, "fd = %d (@%p),\n", *htc->rfd, htc->rfd);
 	VSB_printf(vsb, "doclose = %s,\n", sess_close_2str(htc->doclose, 0));
-	pan_ws(vsb, htc->ws);
+	WS_Panic(htc->ws, vsb);
 	VSB_printf(vsb, "{rxbuf_b, rxbuf_e} = {%p, %p},\n",
 	    htc->rxbuf_b, htc->rxbuf_e);
 	VSB_printf(vsb, "{pipeline_b, pipeline_e} = {%p, %p},\n",
@@ -203,7 +171,7 @@ pan_http(struct vsb *vsb, const char *id, const struct http *h)
 		return;
 	VSB_indent(vsb, 2);
 	PAN_CheckMagic(vsb, h, HTTP_MAGIC);
-	pan_ws(vsb, h->ws);
+	WS_Panic(h->ws, vsb);
 	VSB_cat(vsb, "hdrs {\n");
 	VSB_indent(vsb, 2);
 	for (i = 0; i < h->nhd; ++i) {
@@ -302,7 +270,7 @@ pan_wrk(struct vsb *vsb, const struct worker *wrk)
 		return;
 	VSB_indent(vsb, 2);
 	PAN_CheckMagic(vsb, wrk, WORKER_MAGIC);
-	pan_ws(vsb, wrk->aws);
+	WS_Panic(wrk->aws, vsb);
 
 	m = wrk->cur_method;
 	VSB_cat(vsb, "VCL::method = ");
@@ -424,7 +392,7 @@ pan_busyobj(struct vsb *vsb, const struct busyobj *bo)
 	if (bo->filter_list != NULL)
 		VSB_printf(vsb, "filter_list = \"%s\",\n", bo->filter_list);
 
-	pan_ws(vsb, bo->ws);
+	WS_Panic(bo->ws, vsb);
 	VSB_printf(vsb, "ws_bo = %p,\n", (void *)bo->ws_bo);
 
 	// bereq0 left out
@@ -531,7 +499,7 @@ pan_req(struct vsb *vsb, const struct req *req)
 	if (req->wrk != NULL)
 		pan_wrk(vsb, req->wrk);
 
-	pan_ws(vsb, req->ws);
+	WS_Panic(req->ws, vsb);
 	if (VALID_OBJ(req->htc, HTTP_CONN_MAGIC))
 		pan_htc(vsb, req->htc);
 	pan_http(vsb, "req", req->http);
@@ -582,7 +550,7 @@ pan_sess(struct vsb *vsb, const struct sess *sp)
 	    sp->fd, VXID(sp->vxid));
 	VSB_printf(vsb, "t_open = %f,\n", sp->t_open);
 	VSB_printf(vsb, "t_idle = %f,\n", sp->t_idle);
-	pan_ws(vsb, sp->ws);
+	WS_Panic(sp->ws, vsb);
 	VSB_printf(vsb, "transport = %s",
 	    xp == NULL ? "<none>" : xp->name);
 	if (xp != NULL && xp->sess_panic != NULL) {
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index dca3a5cc2..9d98778d3 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -464,6 +464,8 @@ void VMOD_Panic(struct vsb *);
 void WRK_Init(void);
 
 /* cache_ws.c */
+void WS_Panic(const struct ws *ws, struct vsb *vsb);
+
 void WS_Rollback(struct ws *, uintptr_t);
 void *WS_AtOffset(const struct ws *ws, unsigned off, unsigned len);
 
diff --git a/bin/varnishd/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c
index 82ff49ac8..44610e682 100644
--- a/bin/varnishd/cache/cache_vary.c
+++ b/bin/varnishd/cache/cache_vary.c
@@ -219,9 +219,6 @@ vry_cmp(const uint8_t *v1, const uint8_t *v2)
 
 /**********************************************************************
  * Prepare predictive vary string
- *
- * XXX: Strictly speaking vary_b and vary_e could be replaced with
- * XXX: req->ws->{f,r}.   Space in struct req vs. code-readability...
  */
 
 void
diff --git a/bin/varnishd/cache/cache_ws.c b/bin/varnishd/cache/cache_ws.c
index 7f51b5379..b4dc26435 100644
--- a/bin/varnishd/cache/cache_ws.c
+++ b/bin/varnishd/cache/cache_ws.c
@@ -415,3 +415,35 @@ WS_VSB_finish(struct vsb *vsb, struct ws *ws, size_t *szp)
 		*szp = 0;
 	return (NULL);
 }
+
+/*--------------------------------------------------------------------*/
+
+void
+WS_Panic(const struct ws *ws, struct vsb *vsb)
+{
+
+	VSB_printf(vsb, "ws = %p {\n", ws);
+	if (PAN_already(vsb, ws))
+		return;
+	VSB_indent(vsb, 2);
+	PAN_CheckMagic(vsb, ws, WS_MAGIC);
+	if (ws->id[0] != '\0' && (!(ws->id[0] & 0x20)))	// cheesy islower()
+		VSB_cat(vsb, "OVERFLOWED ");
+	VSB_printf(vsb, "id = \"%s\",\n", ws->id);
+	VSB_printf(vsb, "{s, f, r, e} = {%p", ws->s);
+	if (ws->f >= ws->s)
+		VSB_printf(vsb, ", +%ld", (long) (ws->f - ws->s));
+	else
+		VSB_printf(vsb, ", %p", ws->f);
+	if (ws->r >= ws->s)
+		VSB_printf(vsb, ", +%ld", (long) (ws->r - ws->s));
+	else
+		VSB_printf(vsb, ", %p", ws->r);
+	if (ws->e >= ws->s)
+		VSB_printf(vsb, ", +%ld", (long) (ws->e - ws->s));
+	else
+		VSB_printf(vsb, ", %p", ws->e);
+	VSB_cat(vsb, "},\n");
+	VSB_indent(vsb, -2);
+	VSB_cat(vsb, "},\n");
+}


More information about the varnish-commit mailing list