Address a number of problems with sysctl_vm_zone().

The zone allocator's locks should be leaflocks, meaning that they
should never be held when entering into another subsystem, however
the sysctl grabs the zone global mutex and individual zone mutexes
while holding the lock it calls SYSCTL_OUT which recurses into the
VM subsystem in order to wire user memory to do a safe copy.  This
can block and cause lock order reversals.

To fix this:
  lock zone global.
  get a count of the number of zones.
  unlock global.
  allocate temporary storage.
  format and SYSCTL_OUT the banner.
  lock global.
  traverse list.
    make sure we haven't looped more than the initial count taken
      to avoid overflowing the allocated buffer.
    lock each nodes.
    read values and format into buffer.
    unlock individual node.
  unlock global.
  format and SYSCTL_OUT the rest of the data.
  free storage.
  return.

Other problems included not checking for errors when doing sysctl out
of the column header.  Fixed.

Inconsistant termination of the copied string. Fixed.

Objected to by: des (for not using sbuf)

Since the output is not variable length and I'm actually over
allocating signifigantly and I'd like to get this fixed now, I'll
work on the sbuf convertion at a later date.  I would not object
to someone else taking it upon themselves to convert it to sbuf.
I hold no MAINTIANER rights to this code (for now).

1 files changed, 29 insertions, 11 deletions

diff --git a/sys/vm/vm_zone.c b/sys/vm/vm_zone.c
index 65c10de..4cddadc 100644
--- a/sys/vm/vm_zone.c
+++ b/sys/vm/vm_zone.c
@@ -414,31 +414,49 @@ zfree(vm_zone_t z, void *item)
 static int
 sysctl_vm_zone(SYSCTL_HANDLER_ARGS)
 {
-	int error, len;
-	char tmpbuf[128];
+	int error, len, cnt;
+	const int linesize = 128;	/* conservative */
+	char *tmpbuf, *offset;
 	vm_zone_t z;
 	char *p;
 
+	cnt = 0;
 	mtx_lock(&zone_mtx);
-	len = snprintf(tmpbuf, sizeof(tmpbuf),
+	SLIST_FOREACH(z, &zlist, zent)
+		cnt++;
+	mtx_unlock(&zone_mtx);
+	MALLOC(tmpbuf, char *, (cnt == 0 ? 1 : cnt) * linesize,
+			M_TEMP, M_WAITOK);
+	len = snprintf(tmpbuf, linesize,
 	    "\nITEM            SIZE     LIMIT    USED    FREE  REQUESTS\n\n");
-	error = SYSCTL_OUT(req, tmpbuf, SLIST_EMPTY(&zlist) ? len-1 : len);
+	if (cnt == 0)
+		tmpbuf[len - 1] = '\0';
+	error = SYSCTL_OUT(req, tmpbuf, cnt == 0 ? len-1 : len);
+	if (error || cnt == 0)
+		goto out;
+	offset = tmpbuf;
+	mtx_lock(&zone_mtx);
 	SLIST_FOREACH(z, &zlist, zent) {
+		if (cnt == 0)	/* list may have changed size */
+			break;
 		mtx_lock(&z->zmtx);
-		len = snprintf(tmpbuf, sizeof(tmpbuf),
+		len = snprintf(offset, linesize,
 		    "%-12.12s  %6.6u, %8.8u, %6.6u, %6.6u, %8.8u\n",
 		    z->zname, z->zsize, z->zmax, (z->ztotal - z->zfreecnt),
 		    z->zfreecnt, z->znalloc);
-		for (p = tmpbuf + 12; p > tmpbuf && *p == ' '; --p)
+		mtx_unlock(&z->zmtx);
+		for (p = offset + 12; p > offset && *p == ' '; --p)
 			/* nothing */ ;
 		p[1] = ':';
-		mtx_unlock(&z->zmtx);
-		if (SLIST_NEXT(z, zent) == NULL)
-			tmpbuf[len - 1] = 0;
-		if ((error = SYSCTL_OUT(req, tmpbuf, len)) != 0)
-			break;
+		cnt--;
+		offset += len;
 	}
 	mtx_unlock(&zone_mtx);
+	offset--;
+	*offset = '\0';
+	error = SYSCTL_OUT(req, tmpbuf, offset - tmpbuf);
+out:
+	FREE(tmpbuf, M_TEMP);
 	return (error);
 }