git: a34d3ca6efc4 - main - re: Avoid subobject overread when setting IDRn
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);
/*