Re: kern/56894: if_lagg.c crashes on alignment-picky architectures

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

The following reply was made to PR kern/56894; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: Tom Lane <tgl%sss.pgh.pa.us@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, Shoichi Yamaguchi <yamaguchi%NetBSD.org@localhost>
Subject: Re: kern/56894: if_lagg.c crashes on alignment-picky architectures
Date: Mon, 20 Jun 2022 02:02:21 +0000

 This is a multi-part message in MIME format.
 --=_9iRVuQui7SP0XI0my0i42KB+RbbJU+ZS
 
 Can you try the attached patch and see if it helps?
 
 --=_9iRVuQui7SP0XI0my0i42KB+RbbJU+ZS
 Content-Type: text/plain; charset="ISO-8859-1"; name="lagg-align"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="lagg-align.patch"
 
 From a4ca0a1f76a25d5121cfd5946320655cdf4c7c69 Mon Sep 17 00:00:00 2001
 From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 Date: Mon, 20 Jun 2022 02:00:10 +0000
 Subject: [PATCH] lagg(4): Safely handle misaligned mbufs.
 
 Optimizing for non-strict-alignment architectures -- without falling
 afoul of alignment sanitizers or overeager compilers -- is left as an
 exercise for the reader.
 ---
  sys/net/lagg/if_lagg.c      | 14 ++++++++------
  sys/net/lagg/if_laggproto.h |  5 +++--
  2 files changed, 11 insertions(+), 8 deletions(-)
 
 diff --git a/sys/net/lagg/if_lagg.c b/sys/net/lagg/if_lagg.c
 index 68d2c43663d1..d98bd1c51777 100644
 --- a/sys/net/lagg/if_lagg.c
 +++ b/sys/net/lagg/if_lagg.c
 @@ -895,7 +895,7 @@ lagg_hashmbuf(struct lagg_softc *sc, struct mbuf *m)
  	*(hp) =3D hash32_buf(&(v), sizeof(v), *(hp));	\
  } while(0)
 =20
 -	eh =3D lagg_m_extract(m, 0, sizeof(*eh), &buf);
 +	eh =3D lagg_m_extract(m, 0, sizeof(*eh), __alignof(*eh), &buf);
  	if (eh =3D=3D NULL)
  		goto out;
 =20
 @@ -903,7 +903,8 @@ lagg_hashmbuf(struct lagg_softc *sc, struct mbuf *m)
  	etype =3D ntohs(eh->ether_type);
 =20
  	if (etype =3D=3D ETHERTYPE_VLAN) {
 -		evl =3D lagg_m_extract(m, 0, sizeof(*evl), &buf);
 +		evl =3D lagg_m_extract(m, 0, sizeof(*evl), __alignof(*evl),
 +		    &buf);
  		if (evl =3D=3D NULL)
  			goto out;
 =20
 @@ -924,7 +925,7 @@ lagg_hashmbuf(struct lagg_softc *sc, struct mbuf *m)
 =20
  	switch (etype) {
  	case ETHERTYPE_IP:
 -		ip =3D lagg_m_extract(m, off, sizeof(*ip), &buf);
 +		ip =3D lagg_m_extract(m, off, sizeof(*ip), __alignof(*ip), &buf);
  		if (ip =3D=3D NULL)
  			goto out;
 =20
 @@ -937,7 +938,8 @@ lagg_hashmbuf(struct lagg_softc *sc, struct mbuf *m)
  		proto =3D ip->ip_p;
  		break;
  	case ETHERTYPE_IPV6:
 -		ip6 =3D lagg_m_extract(m, off, sizeof(*ip6), &buf);
 +		ip6 =3D lagg_m_extract(m, off, sizeof(*ip6), __alignof(*ip6),
 +		    &buf);
  		if (ip6 =3D=3D NULL)
  			goto out;
 =20
 @@ -957,7 +959,7 @@ lagg_hashmbuf(struct lagg_softc *sc, struct mbuf *m)
 =20
  	switch (proto) {
  	case IPPROTO_TCP:
 -		th =3D lagg_m_extract(m, off, sizeof(*th), &buf);
 +		th =3D lagg_m_extract(m, off, sizeof(*th), __alignof(*th), &buf);
  		if (th =3D=3D NULL)
  			goto out;
 =20
 @@ -967,7 +969,7 @@ lagg_hashmbuf(struct lagg_softc *sc, struct mbuf *m)
  		}
  		break;
  	case IPPROTO_UDP:
 -		uh =3D lagg_m_extract(m, off, sizeof(*uh), &buf);
 +		uh =3D lagg_m_extract(m, off, sizeof(*uh), __alignof(*uh), &buf);
  		if (uh =3D=3D NULL)
  			goto out;
 =20
 diff --git a/sys/net/lagg/if_laggproto.h b/sys/net/lagg/if_laggproto.h
 index c9732ea52564..7c7497f134bf 100644
 --- a/sys/net/lagg/if_laggproto.h
 +++ b/sys/net/lagg/if_laggproto.h
 @@ -217,7 +217,8 @@ struct lagg_softc {
  	(_lp)->lp_ioctl((_lp)->lp_ifp, (_cmd), (_data))
 =20
  static inline const void *
 -lagg_m_extract(struct mbuf *m, size_t off, size_t reqlen, void *buf)
 +lagg_m_extract(struct mbuf *m, size_t off, size_t reqlen, size_t align,
 +    void *buf)
  {
  	ssize_t len;
  	const void *rv;
 @@ -229,7 +230,7 @@ lagg_m_extract(struct mbuf *m, size_t off, size_t reqle=
 n, void *buf)
  		return NULL;
  	}
 =20
 -	if (m->m_len >=3D len) {
 +	if (m->m_len >=3D len && ((uintptr_t)mtod(m, uint8_t *) % align) =3D=3D 0=
 ) {
  		rv =3D mtod(m, uint8_t *) + off;
  	} else {
  		m_copydata(m, off, reqlen, buf);
 
 --=_9iRVuQui7SP0XI0my0i42KB+RbbJU+ZS--