Author: Kev <klmitch@mit.edu>

Log message:

Remove msgq_map(), since we're using only msgq_mapiov().  Tweak some buffer
length calculations to prevent off-by-one errors.  Unfortunately, these
possible off-by-one errors cannot possibly cause the problems we're seeing,
so add some extra asserts and an incredibly ugly hack to detect the
problem's signature--a pointer transmuted to 0x8000a0d, corresponding to
\r\n\0 (+ first byte of pointer) as stored in memory.

Testing:

The changes compile and run--but we *want* this patch to dump core!  Make
absolutely sure you compile with -g.  Hopefully this'll catch the culprit,
or at least bring us one step closer to finding the problem.

git-svn-id: file:///home/klmitch/undernet-ircu/undernet-ircu-svn/ircu2/trunk@380 c9e4aea6-c8fd-4c43-8297-357d70d61c8c

diff --git a/ChangeLog b/ChangeLog
index f25f609c1180d291311355e443754cd00cc0428f..c710d33e8f0529d7eb437df79dc063b4e049dee3 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2001-01-22  Kevin L. Mitchell  <klmitch@mit.edu>
+
+       * ircd/msgq.c: add an incredibly ugly hack to attempt to track
+       down an apparent buffer overflow; remove msgq_map(), since it's no
+       longer used anywhere; slight tweaks to prevent off-by-one errors,
+       but these can't explain the problems we've seen
+
+       * include/msgq.h: remove msgq_map(), since it's no longer used
+       anywhere
+
 2001-01-18  Kevin L. Mitchell  <klmitch@mit.edu>
 
        * ircd/s_user.c (set_nick_name): call client_set_privs() after

diff --git a/include/msgq.h b/include/msgq.h
index a66e3fc6020e40356fdaca8b6680cadc5331e9c2..ae92211096980258e262e8ccef2d3a4c60fdc604 100644 (file)
--- a/include/msgq.h
+++ b/include/msgq.h
@@ -83,7 +83,6 @@ struct MsgQ {
  */
 extern void msgq_init(struct MsgQ *mq);
 extern void msgq_delete(struct MsgQ *mq, unsigned int length);
-extern const char *msgq_map(const struct MsgQ *mq, unsigned int *length_p);
 extern int msgq_mapiov(const struct MsgQ *mq, struct iovec *iov, int count,
                       unsigned int *len);
 extern struct MsgBuf *msgq_make(struct Client *dest, const char *format, ...);

diff --git a/ircd/msgq.c b/ircd/msgq.c
index bc45233bbd6e730ea128d125090a5d10ab2d43d0..7c03fd9785973a494ef90183dd51619c93f2a991 100644 (file)
--- a/ircd/msgq.c
+++ b/ircd/msgq.c
@@ -52,6 +52,32 @@ static struct {
 struct MsgCounts msgBufCounts = { 0, 0 };
 struct MsgCounts msgCounts = { 0, 0 };
 
+/* XXX HACK HACK HACK XXX */
+#if 1
+/* First, force assertion checking */
+#undef NDEBUG
+#include <assert.h>
+
+/* This routine is TEMPORARY and is intended to track down a problem we're
+ * having with apparent buffer overflows in this file.
+ */
+static void
+msgq_integrity(void)
+{
+  struct MsgBuf *mb;
+  struct Msg *msg;
+
+  for (mb = MQData.msgs; mb; mb = mb->next)
+    assert(((unsigned long)mb) != 0x8000a0d);
+  for (mb = MQData.free_mbs; mb; mb = mb->next)
+    assert(((unsigned long)mb) != 0x8000a0d);
+  for (msg = MQData.free_msgs; msg; msg = msg->next)
+    assert(((unsigned long)msg) != 0x8000a0d);
+}
+#else
+#define msgq_integrity()       ((void)0)
+#endif /* XXX HACK HACK HACK XXX */
+
 /*
  * This routine is used to remove a certain amount of data from a given
  * queue and release the Msg (and MsgBuf) structure if needed
@@ -62,6 +88,8 @@ msgq_delmsg(struct MsgQ *mq, struct MsgQList *qlist, unsigned int *length_p)
   struct Msg *m;
   unsigned int msglen;
 
+  msgq_integrity();
+
   assert(0 != mq);
   assert(0 != qlist);
   assert(0 != qlist->head);
@@ -93,6 +121,8 @@ msgq_delmsg(struct MsgQ *mq, struct MsgQList *qlist, unsigned int *length_p)
     m->sent += *length_p; /* this much of the message has been sent */
     *length_p = 0; /* we've dealt with it all */
   }
+
+  msgq_integrity();
 }
 
 /*
@@ -101,6 +131,8 @@ msgq_delmsg(struct MsgQ *mq, struct MsgQList *qlist, unsigned int *length_p)
 void
 msgq_init(struct MsgQ *mq)
 {
+  msgq_integrity();
+
   assert(0 != mq);
 
   mq->length = 0;
@@ -109,6 +141,8 @@ msgq_init(struct MsgQ *mq)
   mq->queue.tail = 0;
   mq->prio.head = 0;
   mq->prio.tail = 0;
+
+  msgq_integrity();
 }
 
 /*
@@ -120,6 +154,8 @@ msgq_init(struct MsgQ *mq)
 void
 msgq_delete(struct MsgQ *mq, unsigned int length)
 {
+  msgq_integrity();
+
   assert(0 != mq);
 
   while (length > 0) {
@@ -132,35 +168,8 @@ msgq_delete(struct MsgQ *mq, unsigned int length)
     else
       break;
   }
-}
-
-/*
- * This is similiar to the dbuf_map() function to allow us to plug it
- * into the existing code more easily; we may want to have something
- * more fancy in the future that would allow us to make some intelligent
- * use of writev or similiar functions.
- */
-const char *
-msgq_map(const struct MsgQ *mq, unsigned int *length_p)
-{
-  assert(0 != mq);
-  assert(0 != length_p);
 
-  if (mq->length <= 0)
-    return 0;
-
-  if (mq->queue.head && mq->queue.head->sent > 0) { /* partial msg on norm q */
-    *length_p = mq->queue.head->msg->length - mq->queue.head->sent;
-    return mq->queue.head->msg->msg + mq->queue.head->sent;
-  } else if (mq->prio.head) { /* message (partial or complete) on prio queue */
-    *length_p = mq->prio.head->msg->length - mq->prio.head->sent;
-    return mq->prio.head->msg->msg + mq->prio.head->sent;
-  } else if (mq->queue.head) { /* message on normal queue */
-    *length_p = mq->queue.head->msg->length; /* partial already dealt with */
-    return mq->queue.head->msg->msg;
-  }
-
-  return 0; /* shouldn't ever happen */
+  msgq_integrity();
 }
 
 /*
@@ -175,13 +184,17 @@ msgq_mapiov(const struct MsgQ *mq, struct iovec *iov, int count,
   struct Msg *prio;
   int i = 0;
 
+  msgq_integrity();
+
   assert(0 != mq);
   assert(0 != iov);
   assert(0 != count);
   assert(0 != len);
 
-  if (mq->length <= 0) /* no data to map */
+  if (mq->length <= 0) { /* no data to map */
+    msgq_integrity();
     return 0;
+  }
 
   if (mq->queue.head && mq->queue.head->sent > 0) { /* partial msg on norm q */
     iov[i].iov_base = mq->queue.head->msg->msg + mq->queue.head->sent;
@@ -191,8 +204,10 @@ msgq_mapiov(const struct MsgQ *mq, struct iovec *iov, int count,
     queue = mq->queue.head->next; /* where we start later... */
 
     i++; /* filled an iovec... */
-    if (!--count) /* check for space */
+    if (!--count) { /* check for space */
+      msgq_integrity();
       return i;
+    }
   } else
     queue = mq->queue.head; /* start at head of queue */
 
@@ -204,8 +219,10 @@ msgq_mapiov(const struct MsgQ *mq, struct iovec *iov, int count,
     prio = mq->prio.head->next; /* where we start later... */
 
     i++; /* filled an iovec... */
-    if (!--count) /* check for space */
+    if (!--count) { /* check for space */
+      msgq_integrity();
       return i;
+    }
   } else
     prio = mq->prio.head; /* start at head of prio */
 
@@ -215,8 +232,10 @@ msgq_mapiov(const struct MsgQ *mq, struct iovec *iov, int count,
     *len += iov[i].iov_len;
 
     i++; /* filled an iovec... */
-    if (!--count) /* check for space */
+    if (!--count) { /* check for space */
+      msgq_integrity();
       return i;
+    }
   }
 
   for (; queue; queue = queue->next) { /* go through normal queue */
@@ -225,10 +244,14 @@ msgq_mapiov(const struct MsgQ *mq, struct iovec *iov, int count,
     *len += iov[i].iov_len;
 
     i++; /* filled an iovec... */
-    if (!--count) /* check for space */
+    if (!--count) { /* check for space */
+      msgq_integrity();
       return i;
+    }
   }
 
+  msgq_integrity();
+
   return i;
 }
 
@@ -242,6 +265,8 @@ msgq_vmake(struct Client *dest, const char *format, va_list vl)
 {
   struct MsgBuf *mb;
 
+  msgq_integrity();
+
   assert(0 != format);
 
   if (!(mb = MQData.free_mbs)) { /* do I need to allocate one? */
@@ -257,16 +282,20 @@ msgq_vmake(struct Client *dest, const char *format, va_list vl)
   mb->ref = 1;
 
   /* fill the buffer */
-  mb->length = ircd_vsnprintf(dest, mb->msg, sizeof(mb->msg) - 2, format, vl);
+  mb->length = ircd_vsnprintf(dest, mb->msg, sizeof(mb->msg) - 3, format, vl);
 
   mb->msg[mb->length++] = '\r'; /* add \r\n to buffer */
   mb->msg[mb->length++] = '\n';
   mb->msg[mb->length] = '\0'; /* not strictly necessary */
 
+  assert(mb->length < sizeof(mb->msg));
+
   if (MQData.msgs) /* link it into the list */
     MQData.msgs->prev_p = &mb->next;
   MQData.msgs = mb;
 
+  msgq_integrity();
+
   return mb;
 }
 
@@ -291,19 +320,28 @@ msgq_append(struct Client *dest, struct MsgBuf *mb, const char *format, ...)
 {
   va_list vl;
 
+  msgq_integrity();
+
   assert(0 != mb);
   assert(0 != format);
 
+  assert(2 < mb->length);
+  assert(sizeof(mb->msg) > mb->length);
+
   mb->length -= 2; /* back up to before \r\n */
 
   va_start(vl, format); /* append to the buffer */
   mb->length += ircd_vsnprintf(dest, mb->msg + mb->length,
-                              sizeof(mb->msg) - 2 - mb->length, format, vl);
+                              sizeof(mb->msg) - 3 - mb->length, format, vl);
   va_end(vl);
 
   mb->msg[mb->length++] = '\r'; /* add \r\n to buffer */
   mb->msg[mb->length++] = '\n';
   mb->msg[mb->length] = '\0'; /* not strictly necessary */
+
+  assert(mb->length < sizeof(mb->msg));
+
+  msgq_integrity();
 }
 
 /*
@@ -313,6 +351,8 @@ msgq_append(struct Client *dest, struct MsgBuf *mb, const char *format, ...)
 void
 msgq_clean(struct MsgBuf *mb)
 {
+  msgq_integrity();
+
   assert(0 != mb);
   assert(0 < mb->ref);
   assert(0 != mb->prev_p);
@@ -329,6 +369,8 @@ msgq_clean(struct MsgBuf *mb)
 
     msgBufCounts.used--; /* decrement the usage count */
   }
+
+  msgq_integrity();
 }
 
 /*
@@ -340,6 +382,8 @@ msgq_add(struct MsgQ *mq, struct MsgBuf *mb, int prio)
   struct MsgQList *qlist;
   struct Msg *msg;
 
+  msgq_integrity();
+
   assert(0 != mq);
   assert(0 != mb);
   assert(0 < mb->ref);
@@ -374,6 +418,8 @@ msgq_add(struct MsgQ *mq, struct MsgBuf *mb, int prio)
 
   mq->length += mb->length; /* update the queue length */
   mq->count++; /* and the queue count */
+
+  msgq_integrity();
 }
 
 /*
@@ -383,6 +429,8 @@ void
 msgq_count_memory(size_t *msg_alloc, size_t *msg_used, size_t *msgbuf_alloc,
                  size_t *msgbuf_used)
 {
+  msgq_integrity();
+
   assert(0 != msg_alloc);
   assert(0 != msg_used);
   assert(0 != msgbuf_alloc);
@@ -392,6 +440,8 @@ msgq_count_memory(size_t *msg_alloc, size_t *msg_used, size_t *msgbuf_alloc,
   *msg_used = msgCounts.used * sizeof(struct Msg);
   *msgbuf_alloc = msgCounts.alloc * sizeof(struct MsgBuf);
   *msgbuf_used = msgCounts.used * sizeof(struct MsgBuf);
+
+  msgq_integrity();
 }
 
 /*
@@ -402,5 +452,5 @@ msgq_bufleft(struct MsgBuf *mb)
 {
   assert(0 != mb);
 
-  return sizeof(mb->msg) - mb->length; /* the -2 for \r\n is in mb->length */
+  return sizeof(mb->msg) - mb->length - 1; /* \r\n counted in mb->length */
 }