[PATCH v2 0/3] xilinx: board: Improve legacy format handling

[PATCH v2 0/3] xilinx: board: Improve legacy format handling

[Michal Simek]

Hi,

fix and improve legacy format handling to cover cases where eeprom content
is corrupted and random. Very likely detection algorithm can be improved -
for example check that mac address is valid, check all strings, etc. but
the aim of this series is to remove all non printable chars without mac
address and never copy more bytes than expected.

Thanks,
Michal

Changes in v2:
- Add +1 for strlcpy because only size -1 is used compare to strncpy.

Michal Simek (3):
  xilinx: board: Use ETH_ALEN macro for mac address size
  xilinx: board: Fix xilinx_eeprom_legacy_cleanup()
  xilinx: board: Update logic in xilinx_read_eeprom_legacy

 board/xilinx/common/board.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

-- 
2.36.1

[PATCH v2 1/3] xilinx: board: Use ETH_ALEN macro for mac address size

[Michal Simek]

Use predefined macro for eth_mac legacy format.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

(no changes since v1)

 board/xilinx/common/board.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 823d703ea93c..6fc41e978669 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -85,7 +85,7 @@ static struct xilinx_board_description *board_info;
 struct xilinx_legacy_format {
 	char board_sn[18]; /* 0x0 */
 	char unused0[14]; /* 0x12 */
-	char eth_mac[6]; /* 0x20 */
+	char eth_mac[ETH_ALEN]; /* 0x20 */
 	char unused1[170]; /* 0x26 */
 	char board_name[11]; /* 0xd0 */
 	char unused2[5]; /* 0xdc */
-- 
2.36.1

[PATCH v2 2/3] xilinx: board: Fix xilinx_eeprom_legacy_cleanup()

[Michal Simek]

When ethernet mac address contains 0x20 or 0xff MAC address is changed and
bytes are converted to zeros. That's why fix decoding algorithm to ignore
fields where MAC address is stored and all non printable chars (including
space) are zeroed.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

(no changes since v1)

 board/xilinx/common/board.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 6fc41e978669..8bcb54da88e7 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -101,9 +101,13 @@ static void xilinx_eeprom_legacy_cleanup(char *eeprom, int size)
 	for (i = 0; i < size; i++) {
 		byte = eeprom[i];
 
-		/* Remove all ffs and spaces */
-		if (byte == 0xff || byte == ' ')
+		/* Remove all non printable chars but ignore MAC address */
+		if ((i < offsetof(struct xilinx_legacy_format, eth_mac) ||
+		     i >= offsetof(struct xilinx_legacy_format, unused1)) &&
+		     (byte < '!' || byte > '~')) {
 			eeprom[i] = 0;
+			continue;
+		}
 
 		/* Convert strings to lower case */
 		if (byte >= 'A' && byte <= 'Z')
-- 
2.36.1

[PATCH v2 3/3] xilinx: board: Update logic in xilinx_read_eeprom_legacy

[Michal Simek]

When eeprom has random content printing random chars can stuck U-Boot.
That's why update legacy eeprom format decoding algorithm to copy only
maximum amount of chars allocated for fields.
And also print them directly from desc structure.

Previous algorithm was printing strings first directly from eeprom content
and then copy them to desc structure.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v2:
- Add +1 for strlcpy because only size -1 is used compare to strncpy.

 board/xilinx/common/board.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index 8bcb54da88e7..c3b2114ff03a 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -140,21 +140,25 @@ static int xilinx_read_eeprom_legacy(struct udevice *dev, char *name,
 
 	xilinx_eeprom_legacy_cleanup((char *)eeprom_content, size);
 
-	printf("Xilinx I2C Legacy format at %s:\n", name);
-	printf(" Board name:\t%s\n", eeprom_content->board_name);
-	printf(" Board rev:\t%s\n", eeprom_content->board_revision);
-	printf(" Board SN:\t%s\n", eeprom_content->board_sn);
+	/* Terminating \0 chars are the part of desc fields already */
+	strlcpy(desc->name, eeprom_content->board_name,
+		sizeof(eeprom_content->board_name) + 1);
+	strlcpy(desc->revision, eeprom_content->board_revision,
+		sizeof(eeprom_content->board_revision) + 1);
+	strlcpy(desc->serial, eeprom_content->board_sn,
+		sizeof(eeprom_content->board_sn) + 1);
 
 	eth_valid = is_valid_ethaddr((const u8 *)eeprom_content->eth_mac);
 	if (eth_valid)
-		printf(" Ethernet mac:\t%pM\n", eeprom_content->eth_mac);
+		memcpy(desc->mac_addr[0], eeprom_content->eth_mac, ETH_ALEN);
+
+	printf("Xilinx I2C Legacy format at %s:\n", name);
+	printf(" Board name:\t%s\n", desc->name);
+	printf(" Board rev:\t%s\n", desc->revision);
+	printf(" Board SN:\t%s\n", desc->serial);
 
-	/* Terminating \0 chars ensure end of string */
-	strcpy(desc->name, eeprom_content->board_name);
-	strcpy(desc->revision, eeprom_content->board_revision);
-	strcpy(desc->serial, eeprom_content->board_sn);
 	if (eth_valid)
-		memcpy(desc->mac_addr[0], eeprom_content->eth_mac, ETH_ALEN);
+		printf(" Ethernet mac:\t%pM\n", desc->mac_addr);
 
 	desc->header = EEPROM_HEADER_MAGIC;
 
-- 
2.36.1

Re: [PATCH v2 0/3] xilinx: board: Improve legacy format handling

[Michal Simek]

On 1/24/23 16:19, Michal Simek wrote:
> Hi,
> 
> fix and improve legacy format handling to cover cases where eeprom content
> is corrupted and random. Very likely detection algorithm can be improved -
> for example check that mac address is valid, check all strings, etc. but
> the aim of this series is to remove all non printable chars without mac
> address and never copy more bytes than expected.
> 
> Thanks,
> Michal
> 
> Changes in v2:
> - Add +1 for strlcpy because only size -1 is used compare to strncpy.
> 
> Michal Simek (3):
>    xilinx: board: Use ETH_ALEN macro for mac address size
>    xilinx: board: Fix xilinx_eeprom_legacy_cleanup()
>    xilinx: board: Update logic in xilinx_read_eeprom_legacy
> 
>   board/xilinx/common/board.c | 34 +++++++++++++++++++++-------------
>   1 file changed, 21 insertions(+), 13 deletions(-)
> 

Applied.
M