Pad m_hdr on 32bit architectures to to prevent alignment and padding

problems with the way MLEN, MHLEN, and struct mbuf are set up.

CTASSERT's are provided to detect such issues at compile time in the
future.

The #define MLEN and MHLEN calculation do not take actual compiler-
induced alignment and padding inside the complete struct mbuf into
account.  Accordingly appropriate attention is required when changing
members of struct mbuf.

Ideally one would calculate MLEN as (MSIZE - sizeof(((struct mbuf *)0)->m_hdr)
but that doesn't work as the compiler refuses to operate on an as of
yet incomplete structure.

In particular ARM 32bit has more strict alignment requirements which
caused 4 bytes of padding between m_hdr and pkthdr in struct mbuf
because of the 64bit members in pkthdr.  This wasn't picked up by MLEN
and MHLEN causing an overflow of the mbuf provided data storage by
overestimating its size.

I386 didn't show this problem because it handles unaligned access just
fine, albeit at a small performance penalty.

On 64bit architectures the struct mbuf layout is 64bit aligned in all
places.

Reported by:	Thomas Skibo <ThomasSkibo-at-sbcglobal-dot-net>
Tested by:	tuexen, ian, Thomas Skibo (extended patch)
Sponsored by:	The FreeBSD Foundation

2 files changed, 16 insertions, 1 deletions

diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index 98ec889..8e278a4 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -85,6 +85,14 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, m_defragrandomfailures, CTLFLAG_RW,
 #endif
 
 /*
+ * Ensure the correct size of various mbuf parameters.  It could be off due
+ * to compiler-induced padding and alignment artifacts.
+ */
+CTASSERT(sizeof(struct mbuf) == MSIZE);
+CTASSERT(MSIZE - offsetof(struct mbuf, m_dat) == MLEN);
+CTASSERT(MSIZE - offsetof(struct mbuf, m_pktdat) == MHLEN);
+
+/*
  * m_get2() allocates minimum mbuf that would fit "size" argument.
  */
 struct mbuf *
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index ce19715..7e8f427 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -53,6 +53,10 @@
  * externally and attach it to the mbuf in a way similar to that of mbuf
  * clusters.
  *
+ * NB: These calculation do not take actual compiler-induced alignment and
+ * padding inside the complete struct mbuf into account.  Appropriate
+ * attention is required when changing members of struct mbuf.
+ *
  * MLEN is data length in a normal mbuf.
  * MHLEN is data length in an mbuf with pktheader.
  * MINCLSIZE is a smallest amount of data that should be put into cluster.
@@ -84,7 +88,7 @@ struct mb_args {
 
 /*
  * Header present at the beginning of every mbuf.
- * Size ILP32: 20
+ * Size ILP32: 24
  *	 LP64: 32
  */
 struct m_hdr {
@@ -94,6 +98,9 @@ struct m_hdr {
 	int32_t		 mh_len;	/* amount of data in this mbuf */
 	uint32_t	 mh_type:8,	/* type of data in this mbuf */
 			 mh_flags:24;	/* flags; see below */
+#if !defined(__LP64__)
+	uint32_t	 mh_pad;	/* pad for 64bit alignment */
+#endif
 };
 
 /*