d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

re: Avoid subobject overread when setting IDRn

Browse Source 
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
This commit is contained in:
Jessica Clarke 2021-12-23 20:19:31 +00:00
parent 95a70e9ea8
commit a34d3ca6ef
1 changed files with 5 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/dev/re/if_re.c
View File

@ -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);
/*