NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/58702: eqos(4) MAC address fallback is busted



>Number:         58702
>Category:       kern
>Synopsis:       eqos(4) MAC address fallback is busted
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Sep 27 13:30:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, ...
>Organization:
The NetBSD FFFFFFFFFFFE
>Environment:
>Description:
eqos_get_eaddr has a fallback for randomly generating a MAC address, but it can't be taken:

   1265 	maclo = RD4(sc, GMAC_MAC_ADDRESS0_LOW);
   1266 	machi = RD4(sc, GMAC_MAC_ADDRESS0_HIGH) & 0xFFFF;
   1267 	if ((maclo & 0x00000001) != 0) {
   1268 		aprint_error_dev(sc->sc_dev,
   1269 		    "Wrong MAC address. Clearing the multicast bit.\n");
   1270 		maclo &= ~0x00000001;
   1271 	}
   1272 
   1273 	if (maclo == 0xFFFFFFFF && machi == 0xFFFF) {
   1274 		/* Create one */
   1275 		maclo = 0x00f2 | (cprng_strong32() & 0xffff0000);
   1276 		machi = cprng_strong32() & 0xffff;
   1277 	}

https://nxr.netbsd.org/xref/src/sys/dev/ic/dwc_eqos.c?r=1.39#1264

If maclo is 0xffffffff, then the first branch clears the low bit, making it 0xffffffe, so the second branch can never be taken.

Even if it were taken, why do we always set the low 16 bits to be 0x00f2, instead of setting the low 2 bits to be 0x2 (local=1, multicast=0)?  Maybe we should have a random_mac_address routine so these magic constants don't get duplicated everywhere.
>How-To-Repeat:
code inspection
>Fix:
Yes, please!



Home | Main Index | Thread Index | Old Index