kozorizki/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MEDIUM: dns: ensure ring offset is properly reajusted to head

Browse Source 
Since the below patch, ring offset calculation for readers has changed.
  commit d9c718863384e32307f65a9ce319dc362b73feb6
  MEDIUM: ring: make the offset relative to the head/tail instead of absolute

For readers, this requires to adjust their offsets to be relative to the
ring head each time read is resumed. Indeed, buffer head can change any
time a ring_write() is performed after older entries were purged.
This operation was not performed on the DNS code which causes the offset
to become invalid. In most cases, the following BUG_ON() was triggered :

  FATAL: bug condition "msg_len + ofs + cnt + 1 > b_data(buf)" matched
  at src/dns.c:522

Fix this by adjusting DNS reader offsets when entering
dns_session_io_handler() and dns_process_req().

This bug was reproduced by using a backend with 10 servers using SRV
record resolution on a single resolvers section. A BUG_ON() crash would
occur after less than 5 minutes of process execution.

This does not need to be backported as the above patch is not.

This should fix github issue #2068.
This commit is contained in:
Amaury Denoyelle 2023-03-07 11:18:27 +01:00
parent 237e6a0d65
commit 737d10fac1
1 changed files with 16 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/dns.c
View File

@ -474,9 +474,6 @@ static void dns_session_io_handler(struct appctx *appctx)
return;
}
ofs = ds->ofs;
HA_RWLOCK_WRLOCK(DNS_LOCK, &ring->lock);
LIST_DEL_INIT(&appctx->wait_entry);
HA_RWLOCK_WRUNLOCK(DNS_LOCK, &ring->lock);
@ -491,10 +488,10 @@ static void dns_session_io_handler(struct appctx *appctx)
* existing messages before grabbing a reference to a location. This
* value cannot be produced after initialization.
*/
if (unlikely(ofs == ~0)) {
ofs = 0;
if (unlikely(ds->ofs == ~0)) {
ds->ofs = 0;
HA_ATOMIC_INC(b_peek(buf, ofs));
HA_ATOMIC_INC(b_peek(buf, ds->ofs));
}
/* in this loop, ofs always points to the counter byte that precedes
@ -505,6 +502,10 @@ static void dns_session_io_handler(struct appctx *appctx)
/* we were already there, adjust the offset to be relative to
* the buffer's head and remove us from the counter.
*/
ofs = ds->ofs - b_head_ofs(buf);
if (ds->ofs < b_head_ofs(buf))
ofs += b_size(buf);
BUG_ON(ofs >= buf->size);
HA_ATOMIC_DEC(b_peek(buf, ofs));
@ -632,7 +633,7 @@ static void dns_session_io_handler(struct appctx *appctx)
}
HA_ATOMIC_INC(b_peek(buf, ofs));
ds->ofs = ofs;
ds->ofs = b_peek_ofs(buf, ofs);
}
HA_RWLOCK_RDUNLOCK(DNS_LOCK, &ring->lock);
@ -1108,8 +1109,6 @@ static struct task *dns_process_req(struct task *t, void *context, unsigned int
struct dns_session *ds, *ads;
HA_SPIN_LOCK(DNS_LOCK, &dss->lock);
ofs = dss->ofs_req;
HA_RWLOCK_RDLOCK(DNS_LOCK, &ring->lock);
/* explanation for the initialization below: it would be better to do
@ -1120,14 +1119,18 @@ static struct task *dns_process_req(struct task *t, void *context, unsigned int
* existing messages before grabbing a reference to a location. This
* value cannot be produced after initialization.
*/
if (unlikely(ofs == ~0)) {
ofs = 0;
HA_ATOMIC_INC(b_peek(buf, ofs));
if (unlikely(dss->ofs_req == ~0)) {
dss->ofs_req = 0;
HA_ATOMIC_INC(b_peek(buf, dss->ofs_req));
}
/* we were already there, adjust the offset to be relative to
* the buffer's head and remove us from the counter.
*/
ofs = dss->ofs_req - b_head_ofs(buf);
if (dss->ofs_req < b_head_ofs(buf))
ofs += b_size(buf);
BUG_ON(ofs >= buf->size);
HA_ATOMIC_DEC(b_peek(buf, ofs));
@ -1216,7 +1219,7 @@ static struct task *dns_process_req(struct task *t, void *context, unsigned int
}
HA_ATOMIC_INC(b_peek(buf, ofs));
dss->ofs_req = ofs;
dss->ofs_req = b_peek_ofs(buf, ofs);
HA_RWLOCK_RDUNLOCK(DNS_LOCK, &ring->lock);