[PATCH 1/2] board: xilinx: Use strlcat() in board_name_decode()

[Sriram Sriram <sriramsriram@linux.microsoft.com>]

From: Drew Kluemke <ankluemk@microsoft.com>

board_name_decode() assembles the board name into a calloc'd buffer of
MAX_NAME_LENGTH (50) bytes using strcat(). It appends CONFIG_SYS_BOARD
and then, for every detected EEPROM descriptor, "-", desc->name (up to
16 characters), "-rev" and desc->revision (up to 8 characters).

The length check runs only after the loop, so it cannot prevent the
overflow it documents. Each descriptor contributes up to 29 characters,
so a base board plus a carrier card already exceeds the buffer, and the
data being concatenated comes from the EEPROM.

Use strlcat() bounded to MAX_NAME_LENGTH so the writes are truncated
rather than overflowing the allocation. Truncation leaves a string of
MAX_NAME_LENGTH - 1 characters, so relax the check accordingly;
as written it could never fire once strlcat() is used.

Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
---
 board/xilinx/common/board.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index f45b879736e..690991bb435 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -590,38 +590,42 @@ char * __maybe_unused __weak board_name_decode(void)
 
 		/* The first string should be soc name */
 		if (!id)
-			strcat(board_local_name, CONFIG_SYS_BOARD);
+			strlcat(board_local_name, CONFIG_SYS_BOARD,
+				MAX_NAME_LENGTH);
 
 		/*
 		 * For two purpose here:
 		 * soc_name- eg: zynqmp-
 		 * and between base board and CC eg: ..revA-sck...
 		 */
-		strcat(board_local_name, "-");
+		strlcat(board_local_name, "-", MAX_NAME_LENGTH);
 
 		if (desc->name[0]) {
 			/* For DT composition name needs to be lowercase */
 			for (i = 0; i < sizeof(desc->name); i++)
 				desc->name[i] = tolower(desc->name[i]);
 
-			strcat(board_local_name, desc->name);
+			strlcat(board_local_name, desc->name,
+				MAX_NAME_LENGTH);
 		}
 		if (desc->revision[0]) {
-			strcat(board_local_name, "-rev");
+			strlcat(board_local_name, "-rev",
+				MAX_NAME_LENGTH);
 
 			/* And revision needs to be uppercase */
 			for (i = 0; i < sizeof(desc->revision); i++)
 				desc->revision[i] = toupper(desc->revision[i]);
 
-			strcat(board_local_name, desc->revision);
+			strlcat(board_local_name, desc->revision,
+				MAX_NAME_LENGTH);
 		}
 	}
 
 	/*
-	 * Longer strings will end up with buffer overflow and potential
-	 * attacks that's why check it
+	 * Longer strings will be truncated by strlcat, check and
+	 * panic if the source data was too long for the buffer.
 	 */
-	if (strlen(board_local_name) >= MAX_NAME_LENGTH)
+	if (strlen(board_local_name) >= MAX_NAME_LENGTH - 1)
 		panic("Board name can't be determined\n");
 
 	if (strlen(board_local_name))
-- 
2.49.0