Add Session Attribute workspace overflow handling

Notes:

* for the acceptor, I think it makes sense to keep AN assertion (pun!)
  because varnish is not viable if the session workspace is too small
  to even hold the attributes initialized in the acceptor.

  If this was an issue, we should rather revisit the minimum values for
  the session workspace

* for h1 and h2 session setup, I have used XXXAN() because I am not sure
  how we should best handle allocation failures.

* The relevant bit, for now, is the proxy code which may allocate
  arbitrarily long TLV attributes, so this is the code for which we now
  actually handle errors and test that we do

On the vtc: I added the test to o00005.vtc because there existed a
previous overflow test from 267504b8,
but that only tested for the one case of a WS overflow which was already
handled.

Fixes #3145

bin/varnishd/cache/cache_acceptor.c

@@ -317,17 +317,17 @@ vca_mk_tcp(const struct wrk_accept *wa,
 	struct sockaddr_storage ss;
 	socklen_t sl;
 
-	SES_Reserve_remote_addr(sp, &sa);
+	AN(SES_Reserve_remote_addr(sp, &sa));
 	AN(VSA_Build(sa, &wa->acceptaddr, wa->acceptaddrlen));
 	sp->sattr[SA_CLIENT_ADDR] = sp->sattr[SA_REMOTE_ADDR];
 
 	VTCP_name(sa, raddr, VTCP_ADDRBUFSIZE, rport, VTCP_PORTBUFSIZE);
-	SES_Set_String_Attr(sp, SA_CLIENT_IP, raddr);
-	SES_Set_String_Attr(sp, SA_CLIENT_PORT, rport);
+	AN(SES_Set_String_Attr(sp, SA_CLIENT_IP, raddr));
+	AN(SES_Set_String_Attr(sp, SA_CLIENT_PORT, rport));
 
 	sl = sizeof ss;
 	AZ(getsockname(sp->fd, (void*)&ss, &sl));
-	SES_Reserve_local_addr(sp, &sa);
+	AN(SES_Reserve_local_addr(sp, &sa));
 	AN(VSA_Build(sa, &ss, sl));
 	sp->sattr[SA_SERVER_ADDR] = sp->sattr[SA_LOCAL_ADDR];
 	VTCP_name(sa, laddr, VTCP_ADDRBUFSIZE, lport, VTCP_PORTBUFSIZE);
@@ -340,13 +340,13 @@ vca_mk_uds(struct wrk_accept *wa, struct sess *sp, char *laddr, char *lport,
 	struct suckaddr *sa;
 
 	(void) wa;
-	SES_Reserve_remote_addr(sp, &sa);
+	AN(SES_Reserve_remote_addr(sp, &sa));
 	AZ(SES_Set_remote_addr(sp, bogo_ip));
 	sp->sattr[SA_CLIENT_ADDR] = sp->sattr[SA_REMOTE_ADDR];
 	sp->sattr[SA_LOCAL_ADDR] = sp->sattr[SA_REMOTE_ADDR];
 	sp->sattr[SA_SERVER_ADDR] = sp->sattr[SA_REMOTE_ADDR];
-	SES_Set_String_Attr(sp, SA_CLIENT_IP, "0.0.0.0");
-	SES_Set_String_Attr(sp, SA_CLIENT_PORT, "0");
+	AN(SES_Set_String_Attr(sp, SA_CLIENT_IP, "0.0.0.0"));
+	AN(SES_Set_String_Attr(sp, SA_CLIENT_PORT, "0"));
 
 	strcpy(laddr, "0.0.0.0");
 	strcpy(raddr, "0.0.0.0");

bin/varnishd/cache/cache_session.c

@@ -110,8 +110,8 @@ ses_set_attr(const struct sess *sp, enum sess_attr a, const void *src, int sz)
 	return (0);
 }
 
-static void
-ses_reserve_attr(struct sess *sp, enum sess_attr a, void **dst, int sz)
+static int
+ses_res_attr(struct sess *sp, enum sess_attr a, void **dst, int sz)
 {
 	ssize_t o;
 
@@ -120,12 +120,14 @@ ses_reserve_attr(struct sess *sp, enum sess_attr a, void **dst, int sz)
 	assert(sz >= 0);
 	AN(dst);
 	o = WS_ReserveSize(sp->ws, sz);
-	assert(o >= sz);
+	if (o < sz)
+		return (0);
 	*dst = sp->ws->f;
 	o = sp->ws->f - sp->ws->s;
 	WS_Release(sp->ws, sz);
 	assert(o >= 0 && o <= 0xffff);
 	sp->sattr[a] = (uint16_t)o;
+	return (1);
 }
 
 #define SESS_ATTR(UP, low, typ, len)					\
@@ -143,16 +145,16 @@ ses_reserve_attr(struct sess *sp, enum sess_attr a, void **dst, int sz)
 		return (ses_get_attr(sp, SA_##UP, (void**)dst));	\
 	}								\
 									\
-	void								\
+	int								\
 	SES_Reserve_##low(struct sess *sp, typ **dst)			\
 	{								\
 		assert(len > 0);					\
-		ses_reserve_attr(sp, SA_##UP, (void**)dst, len);	\
+		return (ses_res_attr(sp, SA_##UP, (void**)dst, len));	\
 	}
 
 #include "tbl/sess_attr.h"
 
-void
+int
 SES_Set_String_Attr(struct sess *sp, enum sess_attr a, const char *src)
 {
 	void *q;
@@ -164,8 +166,10 @@ SES_Set_String_Attr(struct sess *sp, enum sess_attr a, const char *src)
 	if (strcmp(sess_attr[a].type, "char"))
 		WRONG("wrong sess_attr: not char");
 
-	ses_reserve_attr(sp, a, &q, strlen(src) + 1);
+	if (! ses_res_attr(sp, a, &q, strlen(src) + 1))
+		return (0);
 	strcpy(q, src);
+	return (1);
 }
 
 const char *

bin/varnishd/cache/cache_varnishd.h

@@ -389,9 +389,9 @@ enum htc_status_e HTC_RxStuff(struct http_conn *, htc_complete_f *,
 
 #define SESS_ATTR(UP, low, typ, len)					\
 	int SES_Set_##low(const struct sess *sp, const typ *src);	\
-	void SES_Reserve_##low(struct sess *sp, typ **dst);
+	int SES_Reserve_##low(struct sess *sp, typ **dst);
 #include "tbl/sess_attr.h"
-void SES_Set_String_Attr(struct sess *sp, enum sess_attr a, const char *src);
+int SES_Set_String_Attr(struct sess *sp, enum sess_attr a, const char *src);
 
 /* cache_shmlog.c */
 extern struct VSC_main *VSC_C_main;

bin/varnishd/http1/cache_http1_fsm.c

@@ -112,7 +112,7 @@ http1_new_session(struct worker *wrk, void *arg)
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 
 	HTC_RxInit(req->htc, req->ws);
-	SES_Reserve_proto_priv(sp, &u);
+	XXXAN(SES_Reserve_proto_priv(sp, &u));
 	http1_setstate(sp, H1NEWREQ);
 	wrk->task.func = http1_req;
 	wrk->task.priv = req;

bin/varnishd/http2/cache_http2_session.c

@@ -101,7 +101,7 @@ h2_init_sess(const struct worker *wrk, struct sess *sp,
 
 	if (SES_Get_proto_priv(sp, &up)) {
 		/* Already reserved if we came via H1 */
-		SES_Reserve_proto_priv(sp, &up);
+		XXXAN(SES_Reserve_proto_priv(sp, &up));
 		*up = 0;
 	}
 	if (*up == 0) {
@@ -130,7 +130,7 @@ h2_init_sess(const struct worker *wrk, struct sess *sp,
 		AZ(VHT_Init(h2->dectbl,
 			h2->local_settings.header_table_size));
 
-		SES_Reserve_proto_priv(sp, &up);
+		XXXAN(SES_Reserve_proto_priv(sp, &up));
 		*up = (uintptr_t)h2;
 	}
 	AN(up);

bin/varnishd/proxy/cache_proxy_proto.c

@@ -50,6 +50,13 @@ struct vpx_tlv {
 	char			tlv[1];
 };
 
+static inline int
+vpx_ws_err(const struct req *req)
+{
+	VSL(SLT_Error, req->sp->vxid, "insufficient workspace");
+	return (-1);
+}
+
 /**********************************************************************
  * PROXY 1 protocol
  */
@@ -109,17 +116,23 @@ vpx_proto1(const struct worker *wrk, const struct req *req)
 		return (-1);
 	}
 
-	SES_Reserve_client_addr(req->sp, &sa);
+	if (! SES_Reserve_client_addr(req->sp, &sa))
+		return (vpx_ws_err(req));
+
 	if (VSS_ResolveOne(sa, fld[1], fld[3],
 	    pfam, SOCK_STREAM, AI_NUMERICHOST | AI_NUMERICSERV) == NULL) {
 		VSL(SLT_ProxyGarbage, req->sp->vxid,
 		    "PROXY1: Cannot resolve source address");
 		return (-1);
 	}
-	SES_Set_String_Attr(req->sp, SA_CLIENT_IP, fld[1]);
-	SES_Set_String_Attr(req->sp, SA_CLIENT_PORT, fld[3]);
+	if (! SES_Set_String_Attr(req->sp, SA_CLIENT_IP, fld[1]))
+		return (vpx_ws_err(req));
+	if (! SES_Set_String_Attr(req->sp, SA_CLIENT_PORT, fld[3]))
+		return (vpx_ws_err(req));
+
+	if (! SES_Reserve_server_addr(req->sp, &sa))
+		return (vpx_ws_err(req));
 
-	SES_Reserve_server_addr(req->sp, &sa);
 	if (VSS_ResolveOne(sa, fld[2], fld[4],
 	    pfam, SOCK_STREAM, AI_NUMERICHOST | AI_NUMERICSERV) == NULL) {
 		VSL(SLT_ProxyGarbage, req->sp->vxid,
@@ -377,14 +390,16 @@ vpx_proto2(const struct worker *wrk, struct req *req)
 		/* dst/server */
 		memcpy(&sin4.sin_addr, p + 20, 4);
 		memcpy(&sin4.sin_port, p + 26, 2);
-		SES_Reserve_server_addr(req->sp, &sa);
+		if (! SES_Reserve_server_addr(req->sp, &sa))
+			return (vpx_ws_err(req));
 		AN(VSA_Build(sa, &sin4, sizeof sin4));
 		VTCP_name(sa, ha, sizeof ha, pa, sizeof pa);
 
 		/* src/client */
 		memcpy(&sin4.sin_addr, p + 16, 4);
 		memcpy(&sin4.sin_port, p + 24, 2);
-		SES_Reserve_client_addr(req->sp, &sa);
+		if (! SES_Reserve_client_addr(req->sp, &sa))
+			return (vpx_ws_err(req));
 		AN(VSA_Build(sa, &sin4, sizeof sin4));
 		break;
 	case 0x21:
@@ -403,14 +418,16 @@ vpx_proto2(const struct worker *wrk, struct req *req)
 		/* dst/server */
 		memcpy(&sin6.sin6_addr, p + 32, 16);
 		memcpy(&sin6.sin6_port, p + 50, 2);
-		SES_Reserve_server_addr(req->sp, &sa);
+		if (! SES_Reserve_server_addr(req->sp, &sa))
+			return (vpx_ws_err(req));
 		AN(VSA_Build(sa, &sin6, sizeof sin6));
 		VTCP_name(sa, ha, sizeof ha, pa, sizeof pa);
 
 		/* src/client */
 		memcpy(&sin6.sin6_addr, p + 16, 16);
 		memcpy(&sin6.sin6_port, p + 48, 2);
-		SES_Reserve_client_addr(req->sp, &sa);
+		if (! SES_Reserve_client_addr(req->sp, &sa))
+			return (vpx_ws_err(req));
 		AN(VSA_Build(sa, &sin6, sizeof sin6));
 		break;
 	default:
@@ -422,8 +439,10 @@ vpx_proto2(const struct worker *wrk, struct req *req)
 
 	AN(sa);
 	VTCP_name(sa, hb, sizeof hb, pb, sizeof pb);
-	SES_Set_String_Attr(req->sp, SA_CLIENT_IP, hb);
-	SES_Set_String_Attr(req->sp, SA_CLIENT_PORT, pb);
+	if (! SES_Set_String_Attr(req->sp, SA_CLIENT_IP, hb))
+		return (vpx_ws_err(req));
+	if (! SES_Set_String_Attr(req->sp, SA_CLIENT_PORT, pb))
+		return (vpx_ws_err(req));
 
 	VSL(SLT_Proxy, req->sp->vxid, "2 %s %s %s %s", hb, pb, ha, pa);
 
@@ -450,15 +469,13 @@ vpx_proto2(const struct worker *wrk, struct req *req)
 		return (-1);
 	}
 	tlv = WS_Alloc(req->sp->ws, sizeof *tlv + tlv_len);
-	if (tlv == NULL) {
-		VSL(SLT_ProxyGarbage, req->sp->vxid,
-		    "PROXY2: TLV overflows WS");
-		return (-1);
-	}
+	if (tlv == NULL)
+		return (vpx_ws_err(req));
 	INIT_OBJ(tlv, VPX_TLV_MAGIC);
 	tlv->len = tlv_len;
 	memcpy(tlv->tlv, tlv_start, tlv_len);
-	SES_Reserve_proxy_tlv(req->sp, &up);
+	if (! SES_Reserve_proxy_tlv(req->sp, &up))
+		return (vpx_ws_err(req));
 	*up = (uintptr_t)tlv;
 	return (0);
 }

bin/varnishtest/tests/o00005.vtc

@@ -5,7 +5,8 @@ server s1 {
 	txresp
 } -start
 
-varnish v1 -proto "PROXY" -vcl+backend {
+varnish v1 -arg "-p pool_sess=0,0,0" -proto "PROXY" -vcl+backend {
+	import vtc;
 	import proxy;
 
 	sub vcl_deliver {
@@ -20,6 +21,7 @@ varnish v1 -proto "PROXY" -vcl+backend {
 		set resp.http.key = proxy.cert_key();
 		set resp.http.sign = proxy.cert_sign();
 		set resp.http.cn = proxy.client_cert_cn();
+		set resp.http.ws_free = vtc.workspace_free(session);
 	}
 } -start
 
@@ -243,4 +245,68 @@ client c1 {
 	expect_close
 } -run
 
+delay 1
+
 varnish v1 -expect ws_session_overflow == 1
+
+# error handling elsewhere in the proxy code
+# request is the same as initial
+
+varnish v1 -cliok "param.set workspace_session 450"
+varnish v1 -cliok "param.set pool_sess 10,100,1"
+
+delay 1
+
+# get rid of the surplus session mpl
+client c10 -proxy1 "1.2.3.4:1111 5.6.7.8:5678" {
+	txreq
+	rxresp
+} -start
+client c11 -proxy1 "1.2.3.4:1111 5.6.7.8:5678" {
+	txreq
+	rxresp
+} -start
+client c12 -proxy1 "1.2.3.4:1111 5.6.7.8:5678" {
+	txreq
+	rxresp
+} -start
+client c13 -proxy1 "1.2.3.4:1111 5.6.7.8:5678" {
+	txreq
+	rxresp
+} -start
+client c14 -proxy1 "1.2.3.4:1111 5.6.7.8:5678" {
+	txreq
+	rxresp
+} -start
+
+client c10 -wait
+client c11 -wait
+client c12 -wait
+client c13 -wait
+client c14 -wait
+
+client c2 {
+	# PROXY2 with CRC32C TLV
+	sendhex {
+		0d 0a 0d 0a 00 0d 0a 51 55 49 54 0a
+		21 11 00 65
+		d9 46 b5 21
+		5f 8e a8 22
+		ed 96
+		01 bb
+		03 00 04  95 03 ee 75
+		01 00 02  68 32
+		02 00 0a  68 6f 63 64 65 74 2e 6e 65 74
+		20 00 3d
+		01 00 00 00 00
+		21 00 07  54 4c 53 76 31 2e 33
+		25 00 05  45 43 32 35 36
+		24 00 0a  52 53 41 2d 53 48 41 32 35 36
+		23 00 16  41 45 41 44 2d 41 45 53 31 32 38
+			  2d 47 43 4d 2d 53 48 41 32 35 36
+	}
+	txreq
+	expect_close
+} -run
+
+varnish v1 -expect ws_session_overflow == 2