Mailing List Archive

commit 50780f307600fe14c367892bbd3e9c90535b85a2
Author: Nils Goroll <nils.goroll@uplex.de>
Date: Fri Jan 22 17:33:34 2021 +0100

Fix object access in ESI VFP when there is no object

We set up fetch filters (VFPs) before initializing a storage object to
fetch into. If that fails, we close the filters again.

The esi_gzip VFP interacts with the storage object and was missing
error handling for the case of a teardown with no storage object.

Implementation:

We make sure that for the VFP_ERROR case, we do not interact with the
storage object (which makes no sense if that is going to be C-4'ed any
moment).

vfp_vep_callback() keeps its hands off if (struct vef_priv).error is
set, so we use that to avoid complications in the code.

diff --git a/bin/varnishd/cache/cache_esi_fetch.c b/bin/varnishd/cache/cache_esi_fetch.c
index 0a0d1e2aa..508d46e26 100644
--- a/bin/varnishd/cache/cache_esi_fetch.c
+++ b/bin/varnishd/cache/cache_esi_fetch.c
@@ -120,6 +120,11 @@ vfp_esi_end(struct vfp_ctx *vc, struct vef_priv *vef,
CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
CHECK_OBJ_NOTNULL(vef, VEF_MAGIC);

+ if (retval == VFP_ERROR)
+ vef->error = errno ? errno : EINVAL;
+ else
+ assert(retval == VFP_END);
+
vsb = VEP_Finish(vef->vep);

if (vsb != NULL) {
@@ -137,7 +142,8 @@ vfp_esi_end(struct vfp_ctx *vc, struct vef_priv *vef,
}

if (vef->vgz != NULL) {
- VGZ_UpdateObj(vc, vef->vgz, VUA_END_GZIP);
+ if (retval == VFP_END)
+ VGZ_UpdateObj(vc, vef->vgz, VUA_END_GZIP);
if (VGZ_Destroy(&vef->vgz) != VGZ_END)
retval = VFP_Error(vc,
"ESI+Gzip Failed at the very end");
@@ -292,8 +298,13 @@ vfp_esi_fini(struct vfp_ctx *vc, struct vfp_entry *vfe)
CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC);

- if (vfe->priv1 != NULL)
- (void)vfp_esi_end(vc, vfe->priv1, VFP_ERROR);
+ if (vfe->priv1 == NULL)
+ return;
+
+ if (vc->oc->stobj->stevedore == NULL)
+ errno = ENOMEM;
+
+ (void)vfp_esi_end(vc, vfe->priv1, VFP_ERROR);
vfe->priv1 = NULL;
}

diff --git a/bin/varnishtest/tests/r03502.vtc b/bin/varnishtest/tests/r03502.vtc
new file mode 100644
index 000000000..bde14a2ca
--- /dev/null
+++ b/bin/varnishtest/tests/r03502.vtc
@@ -0,0 +1,56 @@
+varnishtest "#3502 Panic in VEP_Finish() for out-of-storage in vbf_beresp2obj()"
+
+server s1 {
+ # First consume (almost) all of the storage - the value
+ # is brittle, see l1 fail
+ rxreq
+ expect req.url == /url1
+ txresp -bodylen 1048336
+
+ rxreq
+ expect req.http.accept-encoding == gzip
+ txresp -bodylen 1
+} -start
+
+varnish v1 -arg "-sTransient=default,1M -p debug=+syncvsl -p nuke_limit=0" -vcl+backend {
+ sub vcl_recv {
+ if (req.url == "/") {
+ return (pass);
+ }
+ }
+ sub vcl_backend_response {
+ set beresp.http.free = storage.Transient.free_space;
+ if (bereq.url == "/") {
+ set beresp.do_gzip = true;
+ set beresp.do_esi = true;
+ }
+ }
+} -start
+
+logexpect l1 -v v1 -g vxid -q "vxid == 1004" {
+ expect 25 1004 VCL_call {^BACKEND_RESPONSE}
+ expect 0 = BerespHeader {^free:}
+ expect 0 = VCL_return {^deliver}
+ expect 0 = Timestamp {^Process}
+ expect 0 = Filters {^ esi_gzip}
+ expect 0 = BerespUnset {^Content-Length: 1}
+ expect 0 = BerespHeader {^Content-Encoding: gzip}
+ expect 0 = BerespHeader {^Vary: Accept-Encoding}
+ # Ensure the FetchError is in vbf_beresp2obj()
+ # not later in the VFP. Otherwise we have too much free_space
+ fail add = Storage
+ expect 0 = FetchError {^Could not get storage}
+ fail clear
+} -start
+
+client c1 {
+ txreq -url /url1
+ rxresp
+ expect resp.status == 200
+
+ txreq -hdr "Accept-Encoding: gzip"
+ # no storage for synth either
+ expect_close
+} -run
+
+logexpect l1 -wait
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit