[4.1] 9edb2b6 close a potential race which could cause an out-of-bounds array access

PÃ¥l Hermunn Johansen hermunn at varnish-software.com
Wed Aug 10 13:03:08 CEST 2016


commit 9edb2b653f531b9c2456ba4afdf77de327b0153b
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Jul 25 22:41:03 2016 +0200

    close a potential race which could cause an out-of-bounds array access
    
    We're only holding a read lock on the director, but we're updating
    the nxt member concurrently. This should be acceptable as a performance
    tradeoff - the only consequence is that round-robin is not strictly
    going around - it may occasionally skip a backend or hand out the same
    multiple times in a row.
    
    the race is:
    
    	thread	code
    
    	A:	rr->nxt %= rr->vd->n_backend;
    	// rr->nxt == rr->vd->n_backend - 1
    	B:	rr->nxt++;
    	// rr->nxt == rr->vd->n_backend
    	A:	be = rr->vd->backend[nxt];
    	// BOOM
    
    should fix #2024

diff --git a/lib/libvmod_directors/round_robin.c b/lib/libvmod_directors/round_robin.c
index 3aee3ad..c6c9c9b 100644
--- a/lib/libvmod_directors/round_robin.c
+++ b/lib/libvmod_directors/round_robin.c
@@ -62,6 +62,7 @@ vmod_rr_resolve(const struct director *dir, struct worker *wrk,
 	struct vmod_directors_round_robin *rr;
 	unsigned u;
 	VCL_BACKEND be = NULL;
+	unsigned nxt;
 
 	CHECK_OBJ_NOTNULL(dir, DIRECTOR_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -69,8 +70,8 @@ vmod_rr_resolve(const struct director *dir, struct worker *wrk,
 	CAST_OBJ_NOTNULL(rr, dir->priv, VMOD_DIRECTORS_ROUND_ROBIN_MAGIC);
 	vdir_rdlock(rr->vd);
 	for (u = 0; u < rr->vd->n_backend; u++) {
-		rr->nxt %= rr->vd->n_backend;
-		be = rr->vd->backend[rr->nxt];
+		nxt = rr->nxt %= rr->vd->n_backend;
+		be = rr->vd->backend[nxt];
 		rr->nxt++;
 		CHECK_OBJ_NOTNULL(be, DIRECTOR_MAGIC);
 		if (be->healthy(be, bo, NULL))



More information about the varnish-commit mailing list