Skip to content

Commit

Permalink
hash: Close an expiry loophole
Browse files Browse the repository at this point in the history
The opposite of 'EXP_Ttl(req, oc) > req->t_req' should not have been
'EXP_Ttl(NULL, oc) < req->t_req'. If we somehow enter the lookup when
the two operands are equal, the objcore suffers a phenomenon known as
Schrödinger's expiry.

The chances of running into this scenario range from epsilon to 100%.

Because 't_req' is stable across restarts, a soft purge will reliably
trigger this case.

Test case by Alve Elde who first demonstrated the problem.
  • Loading branch information
Dridi committed Apr 22, 2024
1 parent 11aa2c9 commit 08d4e80
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache_hash.c
Expand Up @@ -485,7 +485,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
break;
}

if (EXP_Ttl(NULL, oc) < req->t_req && /* ignore req.ttl */
if (EXP_Ttl(NULL, oc) <= req->t_req && /* ignore req.ttl */
oc->t_origin > exp_t_origin) {
/* record the newest object */
exp_oc = oc;
Expand Down
39 changes: 39 additions & 0 deletions bin/varnishtest/tests/c00132.vtc
@@ -0,0 +1,39 @@
varnishtest "304 revalidations with purge.soft()"

server s1 {
rxreq
txresp -hdr "etag: foo" -body "foo"

rxreq
expect req.http.If-None-Match == "foo"
txresp -status 304 -hdr "etag: foo"
} -start

varnish v1 -vcl+backend {
import purge;

sub vcl_hit {
if (req.restarts == 0) {
purge.soft();
return (restart);
}
}

sub vcl_backend_response {
set beresp.ttl = 1d;
set beresp.grace = 1d;
set beresp.keep = 1d;
}
} -start

client c1 {
txreq
rxresp
expect resp.status == 200
expect resp.body == "foo"

txreq
rxresp
expect resp.status == 200
expect resp.body == "foo"
} -run

0 comments on commit 08d4e80

Please sign in to comment.