git: a34d3ca6efc4 - main - re: Avoid subobject overread when setting IDRn

From: Jessica Clarke <jrtc27_at_FreeBSD.org>
Date: Thu, 23 Dec 2021 20:19:56 UTC
The branch main has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=a34d3ca6efc47eeced62a50a8328a554fa1ad72c

commit a34d3ca6efc47eeced62a50a8328a554fa1ad72c
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2021-12-23 20:19:31 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2021-12-23 20:19:31 +0000

    re: Avoid subobject overread when setting IDRn
    
    IDR0-IDR5 can be read byte-by-byte but must be written to as 4-byte
    words. The current code to do this is rather clunky and ends up reading
    past the end of the union's eaddr member due to MAC addresses only being
    6 bytes. In practice this ends up being fine because the align_dummy
    member will pad the union to a multiple of 4 bytes, but this is dodgy,
    and on CHERI with subobject bounds enforcement enabled, as is done in
    CheriBSD's pure-capability kernel, will trap.
    
    Instead, make the buffer in use the right size, just use an array of
    uint32_t's rather than a char buffer that's then cast to uint32_t * to
    simplify it in the process, and zero-initialise it first to avoid
    reading uninitialised data in the trailing bytes.
    
    Found by:       CHERI
    Reviewed by:    jhb
    Differential Revision:  https://reviews.freebsd.org/D33617
---
 sys/dev/re/if_re.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/sys/dev/re/if_re.c b/sys/dev/re/if_re.c
index cf327932cd74..5d1446e51abf 100644
--- a/sys/dev/re/if_re.c
+++ b/sys/dev/re/if_re.c
@@ -3094,10 +3094,7 @@ re_init_locked(struct rl_softc *sc)
 	struct mii_data		*mii;
 	uint32_t		reg;
 	uint16_t		cfg;
-	union {
-		uint32_t align_dummy;
-		u_char eaddr[ETHER_ADDR_LEN];
-        } eaddr;
+	uint32_t		idr[2];
 
 	RL_LOCK_ASSERT(sc);
 
@@ -3196,12 +3193,11 @@ re_init_locked(struct rl_softc *sc)
 	 * register write enable" mode to modify the ID registers.
 	 */
 	/* Copy MAC address on stack to align. */
-	bcopy(IF_LLADDR(ifp), eaddr.eaddr, ETHER_ADDR_LEN);
+	bzero(idr, sizeof(idr));
+	bcopy(IF_LLADDR(ifp), idr, ETHER_ADDR_LEN);
 	CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_WRITECFG);
-	CSR_WRITE_4(sc, RL_IDR0,
-	    htole32(*(u_int32_t *)(&eaddr.eaddr[0])));
-	CSR_WRITE_4(sc, RL_IDR4,
-	    htole32(*(u_int32_t *)(&eaddr.eaddr[4])));
+	CSR_WRITE_4(sc, RL_IDR0, htole32(idr[0]));
+	CSR_WRITE_4(sc, RL_IDR4, htole32(idr[1]));
 	CSR_WRITE_1(sc, RL_EECMD, RL_EEMODE_OFF);
 
 	/*