1
0
mirror of git://git.gnupg.org/gnupg.git synced 2024-12-23 10:29:58 +01:00

dirmngr: hkp: Avoid potential race condition when some hosts die.

* dirmngr/ks-engine-hkp.c (select_random_host): Use atomic pass
through the host table instead of risking out-of-bounds write.

--

Multiple threads may write to hosttable[x]->dead while
select_random_host() is running.  For example, a housekeeping thread
might clear the ->dead bit on some entries, or another connection to
dirmngr might manually mark a host as alive.

If one or more hosts are resurrected between the two loops over a
given table in select_random_host(), then the allocation of tbl might
not be large enough, resulting in a write past the end of tbl on the
second loop.

This change collapses the two loops into a single loop to avoid this
discrepancy: each host's "dead" bit is now only checked once.

As Werner points out, this isn't currently strictly necessary, since
npth will not switch threads unless a blocking system call is made,
and no blocking system call is made in these two loops.

However, in a subsequent change in this series, we will call a
function in this loop, and that function may sometimes write(2), or
call other functions, which may themselves block.  Keeping this as a
single-pass loop avoids the need to keep track of what might block and
what might not.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>

Gbp-Pq: Topic dirmngr-idling
Gbp-Pq: Name dirmngr-hkp-Avoid-potential-race-condition-when-some.patch
This commit is contained in:
Daniel Kahn Gillmor 2016-10-29 01:25:05 -04:00
parent 337ae2374e
commit 24507b1567

View File

@ -225,29 +225,26 @@ host_in_pool_p (hostinfo_t hi, int tblidx)
static int static int
select_random_host (hostinfo_t hi) select_random_host (hostinfo_t hi)
{ {
int *tbl; int *tbl = NULL;
size_t tblsize; size_t tblsize = 0;
int pidx, idx; int pidx, idx;
/* We create a new table so that we randomly select only from /* We create a new table so that we randomly select only from
currently alive hosts. */ currently alive hosts. */
for (idx = 0, tblsize = 0; for (idx = 0;
idx < hi->pool_len && (pidx = hi->pool[idx]) != -1; idx < hi->pool_len && (pidx = hi->pool[idx]) != -1;
idx++) idx++)
if (hosttable[pidx] && !hosttable[pidx]->dead) if (hosttable[pidx] && !hosttable[pidx]->dead)
{
tblsize++; tblsize++;
tbl = xtryrealloc(tbl, tblsize * sizeof *tbl);
if (!tbl)
return -1; /* memory allocation failed! */
tbl[tblsize-1] = pidx;
}
if (!tblsize) if (!tblsize)
return -1; /* No hosts. */ return -1; /* No hosts. */
tbl = xtrymalloc (tblsize * sizeof *tbl);
if (!tbl)
return -1;
for (idx = 0, tblsize = 0;
idx < hi->pool_len && (pidx = hi->pool[idx]) != -1;
idx++)
if (hosttable[pidx] && !hosttable[pidx]->dead)
tbl[tblsize++] = pidx;
if (tblsize == 1) /* Save a get_uint_nonce. */ if (tblsize == 1) /* Save a get_uint_nonce. */
pidx = tbl[0]; pidx = tbl[0];
else else