⬢ DragonFlyBSD Kernel Audit
DF-2253 / fix.diff
← back to finding ↓ download raw
--- a/sys/libkern/hexncpy.c
+++ b/sys/libkern/hexncpy.c
@@ -53,13 +53,32 @@
 
         /* Save up our base address */
         addr = outb;
-        for (; inlen > 0 && outlen >= MIN_OUTBLEN; --inlen, outlen -= 3) {
+        /*
+         * A converted byte plus its separator (which the terminator below
+         * replaces) costs 3 bytes of outb; without a separator it costs 2.
+         * The old unconditional drain of 3 truncated sep == NULL callers
+         * at two thirds of the buffer (DF-2253).
+         */
+        for (; inlen > 0 && outlen >= MIN_OUTBLEN;
+            --inlen, outlen -= (sep ? 3 : 2)) {
                 *outb++ = hexdigit[*inb >> 4];
                 *outb++ = hexdigit[*inb++ & 0xf];
                 if (sep)
                         *outb++ = *sep;
         }
-        *--outb = 0;
+        /*
+         * Terminate.  With a separator keep the historic behaviour of
+         * replacing the trailing separator with the terminator, but
+         * only when at least one byte was converted.  When nothing was
+         * converted (inlen <= 0) the old unconditional "*--outb" wrote
+         * one byte BEFORE the caller buffer (DF-2252).  Without a
+         * separator the terminator goes right after the last digit
+         * instead of clobbering its low nibble (DF-2253).
+         */
+        if (sep != NULL && outb != addr)
+                *--outb = 0;
+        else
+                *outb = 0;
 
         return addr;
 }
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -2534,6 +2534,17 @@
 	m2 = m;
 	hexstr = kmalloc(HEX_NCPYLEN(len), M_TEMP, M_ZERO | M_WAITOK);
 	while (len) {
+		/*
+		 * Skip empty mbufs: hexncpy() cannot represent
+		 * m_len == 0 and used to underflow / assert-panic
+		 * on them (DF-2252).
+		 */
+		if (m2->m_len == 0) {
+			if (m2->m_next == NULL)
+				break;
+			m2 = m2->m_next;
+			continue;
+		}
 		kprintf("%p %s\n", m2, hexncpy(m2->m_data, m2->m_len, hexstr,
 			HEX_NCPYLEN(m2->m_len), "-"));
 		len -= m2->m_len;