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

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.

diff --git a/src/dns.c b/src/dns.c
index 003f8b9cfc18841603c51e08a95927281c404c77..1e6f287bb52e744153faa17cec28c9508ef5e93d 100644 (file)
--- a/src/dns.c
+++ b/src/dns.c
@@ -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);