Skip to content

Commit

Permalink
close a potential race which could cause an out-of-bounds array access
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nigoroll authored and hermunn committed Aug 10, 2016
1 parent 1b6b671 commit 9edb2b6
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/libvmod_directors/round_robin.c
Expand Up @@ -62,15 +62,16 @@ 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);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
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))
Expand Down

0 comments on commit 9edb2b6

Please sign in to comment.