On Sun, Apr 11, 2021 at 17:40 Tom Rini <tr...@konsulko.com> wrote: > On Mon, Apr 12, 2021 at 12:25:32AM +0200, Heinrich Schuchardt wrote: > > > Hello Andy, > > > > in the code of your patch "mpc85xx: Add support for the Varisys Cyrus > > board" merged in 2015 as 87e29878cab an out of bound access occurs. See > > below. > > > > On 11/4/15 10:48 PM, Andy Fleming wrote: > > > This board runs a P5020 or P5040 chip, and utilizes > > > an EEPROM with similar formatting to the Freescale P5020DS. > > > > > > Large amounts of this code were developed by > > > Adrian Cox <adrian at humboldt dot co dot uk> > > > > > > Signed-off-by: Andy Fleming <aflem...@gmail.com> > > > Reviewed-by: York Sun <york...@freescale.com> > > > --- > > > > <snip /> > > > > > +++ b/board/varisys/common/sys_eeprom.c > > > > <snip /> > > > > > +static struct __attribute__ ((__packed__)) eeprom { > > > + u8 id[4]; /* 0x00 - 0x03 EEPROM Tag 'NXID' */ > > > + u8 sn[12]; /* 0x04 - 0x0F Serial Number */ > > > + u8 errata[5]; /* 0x10 - 0x14 Errata Level */ > > > + u8 date[6]; /* 0x15 - 0x1a Build Date */ > > > + u8 res_0; /* 0x1b Reserved */ > > > + u32 version; /* 0x1c - 0x1f NXID Version */ > > > + u8 tempcal[8]; /* 0x20 - 0x27 Temperature Calibration Factors */ > > > + u8 tempcalsys[2]; /* 0x28 - 0x29 System Temperature Calibration > > Factors */ > > > + u8 tempcalflags; /* 0x2a Temperature Calibration Flags */ > > > + u8 res_1[21]; /* 0x2b - 0x3f Reserved */ > > > + u8 mac_count; /* 0x40 Number of MAC addresses */ > > > + u8 mac_flag; /* 0x41 MAC table flags */ > > > + u8 mac[MAX_NUM_PORTS][6]; /* 0x42 - x MAC addresses */ > > > + u32 crc; /* x+1 CRC32 checksum */ > > > +} e; > > > > <snip /> // MAX_NUM_PORTS = 8 > > > > > +int mac_read_from_eeprom_common(void) > > > +{ > > > + unsigned int i; > > > + u32 crc, crc_offset = offsetof(struct eeprom, crc); > > > + u32 *crcp; /* Pointer to the CRC in the data read from the EEPROM > */ > > > + > > > + puts("EEPROM: "); > > > + > > > + if (read_eeprom()) { > > > + printf("Read failed.\n"); > > > + return 0; > > > + } > > > + > > > + if (!is_valid) { > > > + printf("Invalid ID (%02x %02x %02x %02x)\n", > > > + e.id[0], e.id[1], e.id[2], e.id[3]); > > > + return 0; > > > + } > > > + > > > + crc = crc32(0, (void *)&e, crc_offset); > > > + crcp = (void *)&e + crc_offset; > > > + if (crc != be32_to_cpu(*crcp)) { > > > + printf("CRC mismatch (%08x != %08x)\n", crc, > > > + be32_to_cpu(e.crc)); > > > + return 0; > > > + } > > > + > > > + /* > > > + * MAC address #9 in v1 occupies the same position as the CRC in > v0. > > > + * Erase it so that it's not mistaken for a MAC address. We'll > > > + * update the CRC later. > > > + */ > > > + if (e.version == 0) > > > + memset(e.mac[8], 0xff, 6); > > > > Here you are writing 2 bytes beyond the size of e. > > A useful analysis in case anyone brings these boards back. I'm about to > push the series that delete this due to lack of migration.
Interesting. It’s clear there’s ambiguity there, as some versions have more data there, but that definitely looks wrong. Unfortunately, I don’t have one of these boards anymore (And I doubt nxp does), so no resurrection anytime soon. Andy