[master] f8461d09f Sanitize H2_Send_Frame error handling.
Poul-Henning Kamp
phk at FreeBSD.org
Wed Sep 5 06:43:09 UTC 2018
commit f8461d09f52735279e05f4ac67b416fd9832bae6
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Sep 5 06:40:01 2018 +0000
Sanitize H2_Send_Frame error handling.
The only way H2_Send_Frame can fail is if the TCP connection is
dead, so reach out and set the session errored and be done with it,
rather than pass an inconvenient error status down through the callers.
While here use writev(2) instead of two write(2) calls.
diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 1c28f99ee..70e31d14e 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -217,7 +217,7 @@ h2_error h2h_decode_bytes(struct h2_sess *h2, const uint8_t *ptr,
void H2_Send_Get(struct worker *, struct h2_sess *, struct h2_req *);
void H2_Send_Rel(struct h2_sess *, const struct h2_req *);
-h2_error H2_Send_Frame(struct worker *, const struct h2_sess *,
+void H2_Send_Frame(struct worker *, struct h2_sess *,
h2_frame type, uint8_t flags, uint32_t len, uint32_t stream,
const void *);
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index f6ea07eae..56aa27552 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -132,11 +132,10 @@ h2_connectionerror(uint32_t u)
/**********************************************************************/
-static h2_error
+static void
h2_tx_rst(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2,
uint32_t stream, h2_error h2e)
{
- h2_error ret;
char b[4];
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
@@ -148,11 +147,8 @@ h2_tx_rst(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2,
vbe32enc(b, h2e->val);
H2_Send_Get(wrk, h2, r2);
- ret = H2_Send_Frame(wrk, h2, H2_F_RST_STREAM,
- 0, sizeof b, stream, b);
+ H2_Send_Frame(wrk, h2, H2_F_RST_STREAM, 0, sizeof b, stream, b);
H2_Send_Rel(h2, r2);
-
- return (ret);
}
/**********************************************************************
@@ -936,8 +932,9 @@ h2_procframe(struct worker *wrk, struct h2_sess *h2,
"H2: stream %u: Hit maximum number of "
"concurrent streams", h2->rxf_stream);
// rfc7540,l,1200,1205
- return (h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream,
- H2SE_REFUSED_STREAM));
+ h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream,
+ H2SE_REFUSED_STREAM);
+ return (0);
}
h2->highest_stream = h2->rxf_stream;
r2 = h2_new_req(wrk, h2, h2->rxf_stream, NULL);
@@ -954,7 +951,8 @@ h2_procframe(struct worker *wrk, struct h2_sess *h2,
if (h2->rxf_stream == 0 || h2e->connection)
return (h2e); // Connection errors one level up
- return (h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream, h2e));
+ h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream, h2e);
+ return (0);
}
static int
@@ -1143,8 +1141,8 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
vbe32enc(b, h2->highest_stream);
vbe32enc(b + 4, h2e->val);
H2_Send_Get(wrk, h2, h2->req0);
- (void)H2_Send_Frame(wrk, h2, H2_F_GOAWAY, 0, 8, 0, b);
+ H2_Send_Frame(wrk, h2, H2_F_GOAWAY, 0, 8, 0, b);
H2_Send_Rel(h2, h2->req0);
}
- return (h2e ? 0 : 1);
+ return (h2->error ? 0 : 1);
}
diff --git a/bin/varnishd/http2/cache_http2_send.c b/bin/varnishd/http2/cache_http2_send.c
index 054796e4b..41ca146b6 100644
--- a/bin/varnishd/http2/cache_http2_send.c
+++ b/bin/varnishd/http2/cache_http2_send.c
@@ -29,6 +29,8 @@
#include "config.h"
+#include <sys/uio.h>
+
#include "cache/cache_varnishd.h"
#include "cache/cache_transport.h"
@@ -112,13 +114,14 @@ h2_mk_hdr(uint8_t *hdr, h2_frame ftyp, uint8_t flags,
* the session mtx must be held.
*/
-h2_error
-H2_Send_Frame(struct worker *wrk, const struct h2_sess *h2,
+void
+H2_Send_Frame(struct worker *wrk, struct h2_sess *h2,
h2_frame ftyp, uint8_t flags,
uint32_t len, uint32_t stream, const void *ptr)
{
uint8_t hdr[9];
ssize_t s;
+ struct iovec iov[2];
(void)wrk;
@@ -137,18 +140,24 @@ H2_Send_Frame(struct worker *wrk, const struct h2_sess *h2,
h2->srq->acct.resp_bodybytes += len;
Lck_Unlock(&h2->sess->mtx);
- s = write(h2->sess->fd, hdr, sizeof hdr);
- if (s != sizeof hdr)
- return (H2CE_PROTOCOL_ERROR); // XXX Need private ?
- if (len > 0) {
- s = write(h2->sess->fd, ptr, len);
- if (s != len)
- return (H2CE_PROTOCOL_ERROR); // XXX Need private ?
+ memset(iov, 0, sizeof iov);
+ iov[0].iov_base = hdr;
+ iov[0].iov_len = sizeof hdr;
+ iov[1].iov_base = TRUST_ME(ptr);
+ iov[1].iov_len = len;
+ s = writev(h2->sess->fd, iov, len == 0 ? 1 : 2);
+ if (s != sizeof hdr + len) {
+ /*
+ * There is no point in being nice here, we will be unable
+ * to send a GOAWAY once the code unrolls, so go directly
+ * to the finale and be done with it.
+ */
+ h2->error = H2CE_PROTOCOL_ERROR;
+ } else if (len > 0) {
Lck_Lock(&h2->sess->mtx);
VSLb_bin(h2->vsl, SLT_H2TxBody, len, ptr);
Lck_Unlock(&h2->sess->mtx);
}
- return (0);
}
static int64_t
@@ -251,7 +260,6 @@ void
H2_Send(struct worker *wrk, struct h2_req *r2,
h2_frame ftyp, uint8_t flags, uint32_t len, const void *ptr)
{
- h2_error retval;
struct h2_sess *h2;
uint32_t mfs, tf;
const char *p;
@@ -289,8 +297,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
tf = mfs;
if (len <= tf) {
- (void)H2_Send_Frame(wrk, h2,
- ftyp, flags, len, r2->stream, ptr);
+ H2_Send_Frame(wrk, h2, ftyp, flags, len, r2->stream, ptr);
} else {
AN(ptr);
p = ptr;
@@ -308,19 +315,19 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
assert(VTAILQ_FIRST(&h2->txqueue) == r2);
}
if (tf < len) {
- retval = H2_Send_Frame(wrk, h2, ftyp,
+ H2_Send_Frame(wrk, h2, ftyp,
flags, tf, r2->stream, p);
} else {
if (ftyp->respect_window)
assert(tf == len);
tf = len;
- retval = H2_Send_Frame(wrk, h2, ftyp,
+ H2_Send_Frame(wrk, h2, ftyp,
final_flags, tf, r2->stream, p);
flags = 0;
}
p += tf;
len -= tf;
ftyp = ftyp->continuation;
- } while (len > 0 && retval == 0);
+ } while (!h2->error && len > 0);
}
}
More information about the varnish-commit
mailing list