[master] 1e776c44e ws: Almost ban direct access to the workspace reservation
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Mon Aug 31 18:41:10 UTC 2020
commit 1e776c44e60730f3fef65a5965ba888a1b5289a4
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Wed May 6 15:07:10 2020 +0200
ws: Almost ban direct access to the workspace reservation
With the same exceptions as the front pointer. For the cases where
actually need the value of the reservation pointer, I decided to
change the approach and make it a length computation instead, when
possible.
diff --git a/bin/varnishd/cache/cache_acceptor.c b/bin/varnishd/cache/cache_acceptor.c
index d43144dfc..77dbfaa43 100644
--- a/bin/varnishd/cache/cache_acceptor.c
+++ b/bin/varnishd/cache/cache_acceptor.c
@@ -373,7 +373,7 @@ vca_make_session(struct worker *wrk, void *arg)
VTCP_blocking(wa->acceptsock);
/* Turn accepted socket into a session */
- AN(wrk->aws->r);
+ AN(WS_Reservation(wrk->aws));
sp = SES_New(wrk->pool);
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
wrk->stats->s_sess++;
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index f6b7e6ad0..6284b9f6b 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -1127,6 +1127,6 @@ CNT_Request(struct req *req)
VRB_Free(req);
req->wrk = NULL;
}
- assert(nxt == REQ_FSM_DISEMBARK || req->ws->r == NULL);
+ assert(nxt == REQ_FSM_DISEMBARK || WS_Reservation(req->ws) == NULL);
return (nxt);
}
diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index 147934aa0..5e7994bab 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -270,26 +270,29 @@ HTC_RxStuff(struct http_conn *htc, htc_complete_f *func,
vtim_dur tmo;
vtim_real now;
enum htc_status_e hs;
+ unsigned l, r;
ssize_t z;
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
AN(htc->rfd);
assert(*htc->rfd > 0);
- AN(htc->ws->r);
AN(htc->rxbuf_b);
- assert(htc->rxbuf_b <= htc->rxbuf_e);
- assert(htc->rxbuf_e <= htc->ws->r);
+ AN(WS_Reservation(htc->ws));
+
+ l = pdiff(htc->rxbuf_b, htc->rxbuf_e);
+ r = WS_ReservationSize(htc->ws);
+ assert(l <= r);
AZ(isnan(tn) && isnan(td));
if (t1 != NULL)
assert(isnan(*t1));
- if (htc->rxbuf_e == htc->ws->r) {
+ if (l == r) {
/* Can't work with a zero size buffer */
WS_ReleaseP(htc->ws, htc->rxbuf_b);
return (HTC_S_OVERFLOW);
}
- z = (htc->ws->r - htc->rxbuf_b);
+ z = r;
if (z < maxbytes)
maxbytes = z; /* Cap maxbytes at available WS */
@@ -297,7 +300,8 @@ HTC_RxStuff(struct http_conn *htc, htc_complete_f *func,
now = VTIM_real();
AZ(htc->pipeline_b);
AZ(htc->pipeline_e);
- assert(htc->rxbuf_e <= htc->ws->r);
+ l = pdiff(htc->rxbuf_b, htc->rxbuf_e);
+ assert(l <= r);
hs = func(htc);
if (hs == HTC_S_OVERFLOW || hs == HTC_S_JUNK) {
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index ff4aaa72b..3bb6345e0 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -84,7 +84,7 @@ http1_req(struct worker *wrk, void *arg)
THR_SetRequest(req);
req->transport = &HTTP1_transport;
- AZ(wrk->aws->r);
+ AZ(WS_Reservation(wrk->aws));
HTTP1_Session(wrk, req);
AZ(wrk->v1l);
WS_Assert(wrk->aws);
@@ -311,7 +311,7 @@ HTTP1_Session(struct worker *wrk, struct req *req)
assert(isnan(req->t_req));
AZ(req->vcl);
AZ(req->esi_level);
- AN(req->htc->ws->r);
+ AN(WS_Reservation(req->htc->ws));
hs = HTC_RxStuff(req->htc, HTTP1_Complete,
&req->t_first, &req->t_req,
@@ -319,7 +319,7 @@ HTTP1_Session(struct worker *wrk, struct req *req)
sp->t_idle + SESS_TMO(sp, timeout_idle),
NAN,
cache_param->http_req_size);
- AZ(req->htc->ws->r);
+ AZ(WS_Reservation(req->htc->ws));
if (hs < HTC_S_EMPTY) {
req->acct.req_hdrbytes +=
req->htc->rxbuf_e - req->htc->rxbuf_b;
@@ -354,8 +354,8 @@ HTTP1_Session(struct worker *wrk, struct req *req)
if (H2_prism_complete(req->htc) == HTC_S_COMPLETE) {
if (!FEATURE(FEATURE_HTTP2)) {
SES_Close(req->sp, SC_REQ_HTTP20);
- AZ(req->ws->r);
- AZ(wrk->aws->r);
+ AZ(WS_Reservation(req->ws));
+ AZ(WS_Reservation(wrk->aws));
http1_setstate(sp, H1CLEANUP);
continue;
}
@@ -370,8 +370,8 @@ HTTP1_Session(struct worker *wrk, struct req *req)
if (i) {
assert(req->doclose > 0);
SES_Close(req->sp, req->doclose);
- AZ(req->ws->r);
- AZ(wrk->aws->r);
+ AZ(WS_Reservation(req->ws));
+ AZ(WS_Reservation(wrk->aws));
http1_setstate(sp, H1CLEANUP);
continue;
}
@@ -405,13 +405,13 @@ HTTP1_Session(struct worker *wrk, struct req *req)
AZ(req->top->vcl0);
req->task->func = NULL;
req->task->priv = NULL;
- AZ(req->ws->r);
- AZ(wrk->aws->r);
+ AZ(WS_Reservation(req->ws));
+ AZ(WS_Reservation(wrk->aws));
http1_setstate(sp, H1CLEANUP);
} else if (st == H1CLEANUP) {
- AZ(wrk->aws->r);
- AZ(req->ws->r);
+ AZ(WS_Reservation(wrk->aws));
+ AZ(WS_Reservation(req->ws));
if (sp->fd >= 0 && req->doclose != SC_NULL)
SES_Close(sp, req->doclose);
diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c
index da35661af..7ff71c151 100644
--- a/bin/varnishd/http1/cache_http1_line.c
+++ b/bin/varnishd/http1/cache_http1_line.c
@@ -137,7 +137,7 @@ V1L_Close(struct worker *wrk, uint64_t *cnt)
wrk->v1l = NULL;
CHECK_OBJ_NOTNULL(v1l, V1L_MAGIC);
*cnt = v1l->cnt;
- if (v1l->ws->r)
+ if (WS_Reservation(v1l->ws))
WS_Release(v1l->ws, 0);
WS_Rollback(v1l->ws, v1l->res);
ZERO_OBJ(v1l, sizeof *v1l);
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index f30c02561..0e932d7c9 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -70,9 +70,8 @@ HTTP1_Complete(struct http_conn *htc)
enum htc_status_e retval;
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
-
- assert(htc->rxbuf_e >= htc->rxbuf_b);
- assert(htc->rxbuf_e <= htc->ws->r);
+ AN(WS_Reservation(htc->ws));
+ assert(pdiff(htc->rxbuf_b, htc->rxbuf_e) <= WS_ReservationSize(htc->ws));
/* Skip any leading white space */
for (p = htc->rxbuf_b ; p < htc->rxbuf_e && vct_islws(*p); p++)
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index 34015ac3c..281fc7fac 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -231,13 +231,16 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
struct http *hp;
struct h2h_decode *d;
size_t in_u = 0;
+ const char *r, *e;
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
CHECK_OBJ_NOTNULL(h2->new_req, REQ_MAGIC);
hp = h2->new_req->http;
CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
CHECK_OBJ_NOTNULL(hp->ws, WS_MAGIC);
- AN(hp->ws->r);
+ r = WS_Reservation(hp->ws);
+ AN(r);
+ e = r + WS_ReservationSize(hp->ws);
d = h2->decode;
CHECK_OBJ_NOTNULL(d, H2H_DECODE_MAGIC);
@@ -316,9 +319,9 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
if (d->error == H2SE_ENHANCE_YOUR_CALM) {
d->out = d->reset;
- d->out_l = hp->ws->r - d->out;
+ d->out_l = e - d->out;
d->out_u = 0;
- assert(d->out_u < d->out_l);
+ assert(d->out_l > 0);
} else if (d->error)
break;
}
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 704b5cc94..edcf64c04 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -184,7 +184,7 @@ h2_del_req(struct worker *wrk, const struct h2_req *r2)
/* XXX: PRIORITY reshuffle */
VTAILQ_REMOVE(&h2->streams, r2, list);
Lck_Unlock(&sp->mtx);
- AZ(r2->req->ws->r);
+ AZ(WS_Reservation(r2->req->ws));
Req_Cleanup(sp, wrk, r2->req);
Req_Release(r2->req);
}
@@ -532,7 +532,7 @@ h2_do_req(struct worker *wrk, void *priv)
wrk->stats->client_req++;
if (CNT_Request(req) != REQ_FSM_DISEMBARK) {
- AZ(req->ws->r);
+ AZ(WS_Reservation(req->ws));
AZ(req->top->vcl0);
h2 = r2->h2sess;
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
@@ -563,7 +563,7 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
Lck_Lock(&h2->sess->mtx);
VSLb(h2->vsl, SLT_Debug, "HPACK/FINI %s", h2e->name);
Lck_Unlock(&h2->sess->mtx);
- AZ(r2->req->ws->r);
+ AZ(WS_Reservation(r2->req->ws));
h2_del_req(wrk, r2);
return (h2e);
}
@@ -685,7 +685,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
VSLb(h2->vsl, SLT_Debug, "HPACK(hdr) %s", h2e->name);
Lck_Unlock(&h2->sess->mtx);
(void)h2h_decode_fini(h2);
- AZ(r2->req->ws->r);
+ AZ(WS_Reservation(r2->req->ws));
h2_del_req(wrk, r2);
return (h2e);
}
@@ -720,7 +720,7 @@ h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
VSLb(h2->vsl, SLT_Debug, "HPACK(cont) %s", h2e->name);
Lck_Unlock(&h2->sess->mtx);
(void)h2h_decode_fini(h2);
- AZ(r2->req->ws->r);
+ AZ(WS_Reservation(r2->req->ws));
h2_del_req(wrk, r2);
return (h2e);
}
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index 571a42843..15dd17b27 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -148,7 +148,7 @@ h2_del_sess(struct worker *wrk, struct h2_sess *h2, enum sess_close reason)
VHT_Fini(h2->dectbl);
AZ(pthread_cond_destroy(h2->winupd_cond));
TAKE_OBJ_NOTNULL(req, &h2->srq, REQ_MAGIC);
- AZ(req->ws->r);
+ AZ(WS_Reservation(req->ws));
sp = h2->sess;
Req_Cleanup(sp, wrk, req);
Req_Release(req);
@@ -367,21 +367,21 @@ h2_new_session(struct worker *wrk, void *arg)
HTC_RxPipeline(h2->htc, h2->htc->rxbuf_b + sizeof(H2_prism));
WS_Rollback(h2->ws, 0);
HTC_RxInit(h2->htc, h2->ws);
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
VSLb(h2->vsl, SLT_Debug, "H2: Got pu PRISM");
THR_SetRequest(h2->srq);
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
l = h2_enc_settings(&h2->local_settings, settings, sizeof (settings));
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
H2_Send_Get(wrk, h2, h2->req0);
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
H2_Send_Frame(wrk, h2,
H2_F_SETTINGS, H2FF_NONE, l, 0, settings);
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
H2_Send_Rel(h2, h2->req0);
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
/* and off we go... */
h2->cond = &wrk->cond;
@@ -394,7 +394,7 @@ h2_new_session(struct worker *wrk, void *arg)
h2->error = H2CE_INTERNAL_ERROR;
break;
}
- AN(h2->ws->r);
+ AN(WS_Reservation(h2->ws));
}
AN(h2->error);
diff --git a/bin/varnishd/proxy/cache_proxy_proto.c b/bin/varnishd/proxy/cache_proxy_proto.c
index ff9066e0d..36ffabff2 100644
--- a/bin/varnishd/proxy/cache_proxy_proto.c
+++ b/bin/varnishd/proxy/cache_proxy_proto.c
@@ -480,9 +480,8 @@ vpx_complete(struct http_conn *htc)
char *p, *q;
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
-
- assert(htc->rxbuf_e >= htc->rxbuf_b);
- assert(htc->rxbuf_e <= htc->ws->r);
+ AN(WS_Reservation(htc->ws));
+ assert(pdiff(htc->rxbuf_b, htc->rxbuf_e) <= WS_ReservationSize(htc->ws));
l = htc->rxbuf_e - htc->rxbuf_b;
p = htc->rxbuf_b;
More information about the varnish-commit
mailing list