DF-2934 — m_tag_alloc silently truncates int len into uint16_t m_tag_len

File: sys/kern/uipc_mbuf2.c (pass 2, GLM 5.3) Severity: Info (hardening / latent API trap) · Confidence: certain · No PoC required (Info)

The issue

m_tag_alloc() (sys/kern/uipc_mbuf2.c:256-269) takes the data length as int and allocates len + sizeof(struct m_tag) bytes, but stores the length into struct m_tag.m_tag_len, which is uint16_t (sys/sys/mbuf.h:138-143):

struct m_tag *
m_tag_alloc(uint32_t cookie, int type, int len, int mflags)
{
        struct m_tag *t;

        if (len < 0)
                return NULL;
        t = kmalloc(len + sizeof(struct m_tag), M_PACKET_TAGS, mflags);
        if (t == NULL)
                return NULL;
        t->m_tag_id = type;      /* int  -> uint16_t truncation */
        t->m_tag_len = len;      /* int  -> uint16_t truncation */
        t->m_tag_cookie = cookie;
        return t;
}

For any caller with len > 0xFFFF the allocation is correct but the recorded length wraps (0x10000 + n ⇒ m_tag_len == n). The creator writes len bytes via m_tag_data() while every consumer that trusts m_tag_len (e.g. m_tag_copy() bcopy at uipc_mbuf2.c:352, ng_tag memcmp, netgraph get-hookout responses) under-reads; conversely any consumer that sizes its own accesses off its private constant stays consistent. The mismatch is silent.

The same applies to type (int → uint16_t m_tag_id) and to m_tag_locate(m, cookie, int type, ...) matching (p->m_tag_id == type at uipc_mbuf2.c:335 — a caller passing type = 0x10000 would match a stored id of 0).

Why this is Info, not higher

• Audited every in-tree caller of m_tag_alloc/m_tag_get (21 sites): all pass sizeof(struct …) constants or an already-uint16_t value (ng_tag.c:575 passes uint16_t tag_len; ng_ksocket.c:1113 is bounded by sa_len ≤ 255). No in-tree caller can exceed 0xFFFF today.
• On x86-64 len + sizeof(struct m_tag) cannot wrap (len ≤ INT_MAX after the < 0 check; kmalloc of ~2 GB fails ⇒ NULL), so there is no integer overflow, only the silent length/ID mismatch.
• The m_tag API is EXPORTED for use by klds (mbuf.h:699), so a future or third-party module passing an attacker-influenced int length gets a corrupted tag metadata object with no warning — hence worth the one-line guard.

Recommended fix

--- a/sys/kern/uipc_mbuf2.c
+++ b/sys/kern/uipc_mbuf2.c
@@ -257,8 +257,9 @@ struct m_tag *
 m_tag_alloc(uint32_t cookie, int type, int len, int mflags)
 {
        struct m_tag *t;

-       if (len < 0)
+       if (len < 0 || len > 0xFFFF || type < 0 || type > 0xFFFF)
                return NULL;
        t = kmalloc(len + sizeof(struct m_tag), M_PACKET_TAGS, mflags);

(mirrors struct m_tag field widths; belt-and-braces for exported API).

VERDICT — DF-2934 (m_tag_alloc m_tag_len/m_tag_id truncation)

Status: untested (Info hardening finding — PoC not applicable per audit contract: Low/Info findings skip Phase V). Reproduced: n/a. Impact: none demonstrable in-tree.

Narrative

Pass-2 adversarial re-audit of sys/kern/uipc_mbuf2.c (405 LOC) beyond known DF-0161. The file contains three engines: m_pulldown()/m_dup1() (uipc_mbuf2.c:89-252) and the m_tag engine (uipc_mbuf2.c:254-405).

DF-0161 re-verified STILL PRESENT at sys/kern/uipc_mbuf2.c:376-381 — tprev = t; sits inside the else branch, so every successful copy goes through SLIST_INSERT_HEAD and the destination chain ends up in reverse order. Known finding; not re-reported here.

The one new in-file defect that survived adversarial tracing is the silent truncation in m_tag_alloc() (sys/kern/uipc_mbuf2.c:256-268): int len (and int type) are stored into uint16_t m_tag_len/m_tag_id (sys/sys/mbuf.h:140-141) after allocating len + sizeof(struct m_tag) bytes. For len > 0xFFFF the allocation is full-size but the recorded length wraps, so creator-sized writes and m_tag_len-sized reads disagree.

Why it cannot be reproduced as a PoC on the guest: m_tag_alloc is a kernel-internal (module-exported) API with no syscall/ioctl path that passes a user-controlled length. Every in-tree caller (21 sites audited: ip_encap.c:484, ip_input.c:1837, ip_divert.c:335, ip_carp.c:1669, ip_fw2.c:4289/4347/4414, ip_fw3.c:597, ip_fw3_basic.c:205/470, if.c:2964, pf.c:6496, ip6_input.c:1563, ng_ksocket.c:1113 (bounded by sa_len ≤ 255), ng_ipfw.c:286, ng_lmi.c:332, ng_tag.c:575 (already uint16_t), ieee80211_dragonfly.c:250/664/684) passes a compile-time constant or an already-16-bit value. Netgraph — the only subsystem where a user could shape tag cookies/ids/lengths — gates all control-socket creation behind caps_priv_check(SYSCAP_RESTRICTEDROOT) on DragonFly (sys/netgraph7/socket/ng_socket.c:182-185), so even the related unchecked-m_tag_len consumer bugs (ng_tag.c:537 memcmp, ng_ksocket.c:904-907 stag->id read, if.c:2956 *(int *)(mtag+1) read) are root-only on this platform. Those belong to their own files' audits; recorded here as cross-references.

Classes hunted and killed (pass-2 depth, with citations)

1. m_dup1/m_getl under-provision ⇒ heap overflow: MINCLSIZE == MHLEN+1 (sys/sys/mbuf.h:64), so m_getl never returns an mbuf smaller than len for len ≤ MCLBYTES; both callers cap len > MCLBYTES (uipc_mbuf2.c:99, 241-242). Dead.
2. Shared-cluster write corruption in m_pulldown easy cases: M_LEADINGSPACE/M_TRAILINGSPACE are writability-guarded (sys/sys/mbuf.h:444-463): M_EXT_WRITABLE(m) == (m_sharecount(m) == 1) (mbuf.h:431-432) and m_sharecount returns 99 for custom ext buffers (uipc_mbuf.c), so shared clusters present 0 leading/trailing space and every in-cluster bcopy/m_copydata lands in exclusively-owned memory. The hard-way path only writes into a freshly allocated mbuf. Dead.
3. Tag UAF/double-free via copied chains: copies are deep (bcopy(t+1, p+1, t->m_tag_len), uipc_mbuf2.c:352); m_free frees the chain exactly once (uipc_mbuf.c:1345) and both the pkthdr objcache ctor (uipc_mbuf.c:597) and the free path (uipc_mbuf.c:1361) re-init the SLIST. Dead.
4. m_tag_copy_chain mid-chain kmalloc failure: partial copy is freed via m_tag_delete_chain(to) (uipc_mbuf2.c:372-374) after destination tags were already cleared at entry (369). No leak, no dangling. (Order reversal = DF-0161, known.)
5. m_tag_alloc integer overflow: len < 0 rejected (uipc_mbuf2.c:260-261); 64-bit kmalloc arithmetic cannot wrap for int len. Only the 16-bit truncation remains (this finding).
6. Zone exhaustion / M_DONTWAIT propagation: mflags passed through verbatim to kmalloc (uipc_mbuf2.c:262); all softirq-context callers use M_NOWAIT and check NULL. Dead.
7. Concurrent prepend/locate on the SLIST: engine takes no lock — safe under DragonFly's single-owner mbuf discipline (mbufs migrate between serializers/netisr, never shared between CPUs); no in-tree dual-consumer of one mbuf's tag list found. Dead.
8. m_tag_locate ID collisions: ABI_COMPAT (cookie 0) ids are centrally allocated PACKET_TAG_* (sys/sys/mbuf.h:679-696); no two in-tree subsystems share a cookie+id. Dead.
9. m_pulldown negative off/len: requires caller-supplied negative offset; all in-tree callers derive offsets from validated header fields (ip6.h:319-350, if_pfsync.c). Caller-contract, dead.
10. Jumbo-cluster spurious drop: m_dup1 refuses len > MCLBYTES (uipc_mbuf2.c:241) so a 9K-cluster rest-dup (offp==NULL path or shared cluster path) frees the packet instead of pulling up — a functional drop inherited from KAME/FreeBSD, not memory unsafety. Noted, not filed.
11. Mid-chain M_PKTHDR from m_dup1 flags propagation (uipc_mbuf2.c:243 passes m->m_flags to m_getl, which sets M_PKTHDR on the copy while m_dup_pkthdr is skipped when off != 0): mbuf invariant wart inherited verbatim from FreeBSD; ctor-initialized empty tags keep it memory-safe. Noted, not filed.
12. m_tag_locate/first/next lack the M_PKTHDR KASSERT their siblings (prepend/unlink/delete/delete_chain) have (uipc_mbuf2.c:325-340, 393-405); no in-tree caller passes a non-head mbuf. Hardening footnote.
13. m_tag_unlink on a tag not on m's list ⇒ SLIST_REMOVE NULL walk — caller-contract, no in-tree offender. Dead.

Bottom line

File is otherwise clean at depth; DF-0161 remains the only exploitable-shape bug (Low, known). DF-2934 is a one-line hardening fix on an exported API with zero in-tree exploitants today.