* [PATCH 1/3] xilinx: fru: Add bounds checks when parsing FRU data
2026-10-20 13:44 [PATCH 0/3] xilinx: fru: harden FRU parsing against malformed data Michal Simek
@ 2026-10-20 13:44 ` Michal Simek
2026-10-20 13:44 ` [PATCH 2/3] xilinx: fru: Fix wrong argv index in do_fru_capture() addr check Michal Simek
2026-10-20 13:44 ` [PATCH 3/3] xilinx: fru: Use sizeof() for board header size Michal Simek
2 siblings, 0 replies; 4+ messages in thread
From: Michal Simek @ 2026-10-20 13:44 UTC (permalink / raw)
To: u-boot, git
Cc: Pranav Tilak, Akshay Belsare, Padmarao Begari, Peter Robinson,
Siva Durga Prasad Paladugu, Sriram Sriram, Tom Rini
From: Pranav Tilak <pranav.vinaytilak@amd.com>
fru_capture() reads hdr->off_board and hdr->off_multirec and uses
them to locate FRU areas without checking whether the resulting
offsets are within the captured buffer. The board and multirecord
parsers then trust lengths read from the FRU contents, which can
cause reads beyond the end of the buffer when processing a malformed
or corrupted FRU image.
Pass the buffer size through the parsing path and validate that FRU
headers, areas and fields fit within the supplied buffer, and that
the board area meets the FRU spec's minimum size, before accessing
them. board.c passes the size of the probed EEPROM; the fru capture
command now requires an explicit <size> argument.
Fixes: f1b97b5f18a8 ("xilinx: cmd: Add support for FRU commands")
Signed-off-by: Pranav Tilak <pranav.vinaytilak@amd.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
---
board/xilinx/common/board.c | 7 ++-
board/xilinx/common/fru.c | 12 +++--
board/xilinx/common/fru.h | 5 +-
board/xilinx/common/fru_ops.c | 98 ++++++++++++++++++++++++++++-------
4 files changed, 98 insertions(+), 24 deletions(-)
diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
index ea438472696d..790494f9b388 100644
--- a/board/xilinx/common/board.c
+++ b/board/xilinx/common/board.c
@@ -234,7 +234,12 @@ static int xilinx_read_eeprom_fru(struct udevice *dev, char *name,
goto end;
}
- fru_capture((unsigned long)fru_content);
+ ret = fru_capture((unsigned long)fru_content, eeprom_size);
+ if (ret) {
+ printf("%s: FRU capture failed\n", __func__);
+ goto end;
+ }
+
if (gd->flags & GD_FLG_RELOC || (_DEBUG && IS_ENABLED(CONFIG_DTB_RESELECT))) {
printf("Xilinx I2C FRU format at %s:\n", name);
ret = fru_display(0);
diff --git a/board/xilinx/common/fru.c b/board/xilinx/common/fru.c
index 8cf307e33f20..de165c548592 100644
--- a/board/xilinx/common/fru.c
+++ b/board/xilinx/common/fru.c
@@ -14,6 +14,7 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc,
{
unsigned long addr;
char *endp;
+ u32 size;
if (argc < cmdtp->maxargs)
return CMD_RET_USAGE;
@@ -22,7 +23,11 @@ static int do_fru_capture(struct cmd_tbl *cmdtp, int flag, int argc,
if (*argv[1] == 0 || *endp != 0)
return -1;
- return fru_capture(addr);
+ size = hextoul(argv[3], &endp);
+ if (*argv[3] == 0 || *endp != 0)
+ return -1;
+
+ return fru_capture(addr, size);
}
static int do_fru_display(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -46,7 +51,7 @@ static int do_fru_generate(struct cmd_tbl *cmdtp, int flag, int argc,
}
static struct cmd_tbl cmd_fru_sub[] = {
- U_BOOT_CMD_MKENT(capture, 3, 0, do_fru_capture, "", ""),
+ U_BOOT_CMD_MKENT(capture, 4, 0, do_fru_capture, "", ""),
U_BOOT_CMD_MKENT(display, 2, 0, do_fru_display, "", ""),
U_BOOT_CMD_MKENT(board_gen, 8, 0, do_fru_generate, "", ""),
};
@@ -72,7 +77,8 @@ static int do_fru(struct cmd_tbl *cmdtp, int flag, int argc,
/***************************************************/
U_BOOT_LONGHELP(fru,
- "capture <addr> - Parse and capture FRU table present at address.\n"
+ "capture <addr> <size> - Parse and capture FRU table present at\n"
+ " <addr>. <size> is the buffer length in bytes.\n"
"fru display - Displays content of FRU table that was captured using\n"
" fru capture command\n"
"fru board_gen <addr> <manufacturer> <board name> <serial number>\n"
diff --git a/board/xilinx/common/fru.h b/board/xilinx/common/fru.h
index 2b3fa05a6136..f61c3b31df72 100644
--- a/board/xilinx/common/fru.h
+++ b/board/xilinx/common/fru.h
@@ -98,8 +98,11 @@ struct fru_table {
#define FRU_TYPELEN_TYPE_BINARY 0
#define FRU_TYPELEN_TYPE_ASCII8 3
+/* FRU spec minimum board area size */
+#define FRU_BOARD_AREA_MIN_LEN 16
+
int fru_display(int verbose);
-int fru_capture(unsigned long addr);
+int fru_capture(unsigned long addr, u32 size);
int fru_generate(unsigned long addr, char *manufacturer, char *board_name,
char *serial_no, char *part_no, char *revision);
u8 fru_checksum(u8 *addr, u8 len);
diff --git a/board/xilinx/common/fru_ops.c b/board/xilinx/common/fru_ops.c
index 24b135f7666c..fb88c21cdf6f 100644
--- a/board/xilinx/common/fru_ops.c
+++ b/board/xilinx/common/fru_ops.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
* (C) Copyright 2019 - 2022, Xilinx, Inc.
- * (C) Copyright 2022 - 2023, Advanced Micro Devices, Inc.
+ * (C) Copyright 2022 - 2026, Advanced Micro Devices, Inc.
*/
#include <cpu_func.h>
@@ -166,19 +166,37 @@ int fru_generate(unsigned long addr, char *manufacturer, char *board_name,
return 0;
}
-static int fru_parse_board(unsigned long addr)
+static int fru_parse_board(unsigned long addr, u32 size)
{
+ u8 *data, *term, *limit, *next_addr, *eof, *end;
+ u32 area_len;
u8 i, type;
int len;
- u8 *data, *term, *limit, *next_addr, *eof;
+
+ if (size < sizeof(struct fru_board_info_header)) {
+ printf("%s Board area header out of range\n", __func__);
+ return -EINVAL;
+ }
memcpy(&fru_data.brd.ver, (void *)addr, 6);
+ area_len = fru_data.brd.len * FRU_COMMON_HDR_LEN_MULTIPLIER;
+ if (area_len > size) {
+ printf("%s Board area length out of range\n", __func__);
+ return -EINVAL;
+ }
+
+ if (area_len < FRU_BOARD_AREA_MIN_LEN) {
+ printf("%s Board area too small\n", __func__);
+ return -EINVAL;
+ }
+
/*
* eof marks the last data byte (without checksum). That's why checksum
* is address length - 1 and last data byte is length - 2.
*/
- eof = (u8 *)(fru_data.brd.len * 8 + addr - 2);
+ eof = (u8 *)(area_len + addr - 2);
+ end = (u8 *)addr + size;
addr += 6;
data = (u8 *)&fru_data.brd.manufacturer_type_len;
@@ -187,25 +205,32 @@ static int fru_parse_board(unsigned long addr)
limit = (u8 *)&fru_data.brd + sizeof(struct fru_board_data);
for (i = 0; ; i++, data += FRU_BOARD_MAX_LEN) {
- len = fru_check_type_len(*(u8 *)addr, fru_data.brd.lang_code,
- &type);
- next_addr = (u8 *)addr + 1;
-
- if ((u8 *)addr >= eof) {
+ if ((u8 *)addr >= eof || (u8 *)addr >= end) {
debug("Reach EOF record: addr %lx, eof %lx\n", addr,
(unsigned long)eof);
break;
}
+ len = fru_check_type_len(*(u8 *)addr, fru_data.brd.lang_code,
+ &type);
+ next_addr = (u8 *)addr + 1;
+
/*
* Stop capture if the type is ASCII and valid field length
* is 1 (0xc1) and next FRU data is less than 0x20 (space " ")
- * or it is 0x7f (delete 'DEL').
+ * or it is 0x7f (delete 'DEL'). Only dereference next_addr
+ * if it is still within the buffer.
*/
- if (type == FRU_TYPELEN_TYPE_ASCII8 && len == 1 &&
+ if (type == FRU_TYPELEN_TYPE_ASCII8 && len == 1 &&
+ next_addr < end &&
(*next_addr < 0x20 || *next_addr == 0x7F))
break;
+ if (next_addr + len > end) {
+ debug("%s Board area field out of range\n", __func__);
+ break;
+ }
+
/* Stop when amount of chars is more then fields to record */
if (data + len > limit)
break;
@@ -235,7 +260,7 @@ static int fru_parse_board(unsigned long addr)
return 0;
}
-static int fru_parse_multirec(unsigned long addr)
+static int fru_parse_multirec(unsigned long addr, u32 size)
{
struct fru_multirec_hdr mrc;
u8 checksum = 0;
@@ -245,6 +270,11 @@ static int fru_parse_multirec(unsigned long addr)
debug("%s: multirec addr %lx\n", __func__, addr);
do {
+ if (size < hdr_len) {
+ printf("%s MultiRecord area out of range\n", __func__);
+ return -EINVAL;
+ }
+
memcpy(&mrc.rec_type, (void *)addr, hdr_len);
checksum = fru_checksum((u8 *)addr, hdr_len);
@@ -253,6 +283,12 @@ static int fru_parse_multirec(unsigned long addr)
return -EINVAL;
}
+ if (mrc.len > size - hdr_len) {
+ printf("%s MultiRecord length out of range\n",
+ __func__);
+ return -EINVAL;
+ }
+
if (mrc.rec_type == FRU_MULTIREC_TYPE_OEM) {
struct fru_multirec_mac *mac = (void *)addr + hdr_len;
u32 type = FRU_DUT_MACID;
@@ -271,16 +307,24 @@ static int fru_parse_multirec(unsigned long addr)
}
}
addr += mrc.len + hdr_len;
+ size -= mrc.len + hdr_len;
} while (!(mrc.type & FRU_LAST_REC));
return 0;
}
-int fru_capture(unsigned long addr)
+int fru_capture(unsigned long addr, u32 size)
{
struct fru_common_hdr *hdr;
u8 checksum = 0;
unsigned long multirec_addr = addr;
+ u16 offset;
+ int ret;
+
+ if (size < sizeof(struct fru_common_hdr)) {
+ printf("%s Common header out of range\n", __func__);
+ return -EINVAL;
+ }
checksum = fru_checksum((u8 *)addr, sizeof(struct fru_common_hdr));
if (checksum) {
@@ -293,18 +337,34 @@ int fru_capture(unsigned long addr)
memcpy((void *)&fru_data, (void *)hdr,
sizeof(struct fru_common_hdr));
- fru_data.captured = true;
-
if (hdr->off_board) {
- addr += fru_cal_area_len(hdr->off_board);
- fru_parse_board(addr);
+ offset = fru_cal_area_len(hdr->off_board);
+ if (offset >= size) {
+ printf("%s Board area offset out of range\n", __func__);
+ return -EINVAL;
+ }
+
+ addr += offset;
+ ret = fru_parse_board(addr, size - offset);
+ if (ret)
+ return ret;
}
+ fru_data.captured = true;
env_set_hex("fru_addr", addr);
if (hdr->off_multirec) {
- multirec_addr += fru_cal_area_len(hdr->off_multirec);
- fru_parse_multirec(multirec_addr);
+ offset = fru_cal_area_len(hdr->off_multirec);
+ if (offset >= size) {
+ debug("%s MultiRecord area offset out of range\n",
+ __func__);
+ return 0;
+ }
+
+ multirec_addr += offset;
+ ret = fru_parse_multirec(multirec_addr, size - offset);
+ if (ret)
+ debug("%s MultiRecord parse failed\n", __func__);
}
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread