AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/amdgpu: fix potential overflow in fs_info.debugfs_name
@ 2026-05-18  9:40 Stanley.Yang
  2026-05-18  9:40 ` [PATCH 2/5] drm/amdgpu: init locals in umc_v12_0_convert_error_address Stanley.Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Stanley.Yang @ 2026-05-18  9:40 UTC (permalink / raw)
  To: amd-gfx; +Cc: Hawking.Zhang, Tao.Zhou1, YiPeng.Chai, Candice.Li, Stanley.Yang

Use snprintf() with sizeof(fs_info.debugfs_name) so a long RAS block
name plus the "_err_inject" suffix cannot overflow the 32-byte buffer.

Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index c38af6d3599e..57f13ad5605a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2280,7 +2280,8 @@ void amdgpu_ras_debugfs_create_all(struct amdgpu_device *adev)
 	list_for_each_entry(obj, &con->head, node) {
 		if (amdgpu_ras_is_supported(adev, obj->head.block) &&
 			(obj->attr_inuse == 1)) {
-			sprintf(fs_info.debugfs_name, "%s_err_inject",
+			snprintf(fs_info.debugfs_name, sizeof(fs_info.debugfs_name),
+					"%s_err_inject",
 					get_ras_block_str(&obj->head));
 			fs_info.head = obj->head;
 			amdgpu_ras_debugfs_create(adev, &fs_info, dir);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/5] drm/amdgpu: init locals in umc_v12_0_convert_error_address
  2026-05-18  9:40 [PATCH 1/5] drm/amdgpu: fix potential overflow in fs_info.debugfs_name Stanley.Yang
@ 2026-05-18  9:40 ` Stanley.Yang
  2026-05-18  9:40 ` [PATCH 3/5] drm/amd/ras: cap pending_ecc_list size Stanley.Yang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Stanley.Yang @ 2026-05-18  9:40 UTC (permalink / raw)
  To: amd-gfx; +Cc: Hawking.Zhang, Tao.Zhou1, YiPeng.Chai, Candice.Li, Stanley.Yang

row, col, col_lower, row_lower, row_high and bank could be read on
code paths that never assign them. Initialize them to 0.

Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/umc_v12_0.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
index 6cf674dfc8c4..a6df6a778f50 100644
--- a/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/umc_v12_0.c
@@ -285,7 +285,8 @@ static int umc_v12_0_convert_error_address(struct amdgpu_device *adev,
 					struct ta_ras_query_address_output *addr_out,
 					bool dump_addr)
 {
-	uint32_t col, col_lower, row, row_lower, row_high, bank;
+	uint32_t row = 0, row_lower = 0, row_high = 0;
+	uint32_t col = 0, col_lower = 0, bank = 0;
 	uint32_t channel_index = 0, umc_inst = 0;
 	uint32_t i, bit_num, retire_unit, *flip_bits;
 	uint64_t soc_pa, column, err_addr;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/5] drm/amd/ras: cap pending_ecc_list size
  2026-05-18  9:40 [PATCH 1/5] drm/amdgpu: fix potential overflow in fs_info.debugfs_name Stanley.Yang
  2026-05-18  9:40 ` [PATCH 2/5] drm/amdgpu: init locals in umc_v12_0_convert_error_address Stanley.Yang
@ 2026-05-18  9:40 ` Stanley.Yang
  2026-05-18  9:40 ` [PATCH 4/5] drm/amd/ras: snapshot remote cmd header to fix double-fetch Stanley.Yang
  2026-05-18  9:40 ` [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded helpers Stanley.Yang
  3 siblings, 0 replies; 6+ messages in thread
From: Stanley.Yang @ 2026-05-18  9:40 UTC (permalink / raw)
  To: amd-gfx; +Cc: Hawking.Zhang, Tao.Zhou1, YiPeng.Chai, Candice.Li, Stanley.Yang

Drop new entries once pending_ecc_count hits RAS_UMC_PENDING_ECC_MAX
(4096) so an ECC storm or repeated UMC error injection cannot exhaust
kernel memory. Dropped events are counted and reported via a
rate-limited warning.

Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
---
 drivers/gpu/drm/amd/ras/ras_mgr/ras_sys.h |  9 ++++++
 drivers/gpu/drm/amd/ras/rascore/ras_umc.c | 35 +++++++++++++++++++++++
 drivers/gpu/drm/amd/ras/rascore/ras_umc.h | 12 ++++++++
 3 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/ras/ras_mgr/ras_sys.h b/drivers/gpu/drm/amd/ras/ras_mgr/ras_sys.h
index 8156531a7b63..f34dda7ce87b 100644
--- a/drivers/gpu/drm/amd/ras/ras_mgr/ras_sys.h
+++ b/drivers/gpu/drm/amd/ras/ras_mgr/ras_sys.h
@@ -46,6 +46,15 @@
 			printk(KERN_WARNING fmt, ##__VA_ARGS__);                           \
 	} while (0)
 
+#define RAS_DEV_WARN_RATELIMITED(device, fmt, ...)                                   \
+	do {                                                                       \
+		if (device)                                                              \
+			dev_warn_ratelimited(((struct amdgpu_device *)device)->dev,        \
+				fmt, ##__VA_ARGS__);                                            \
+		else                                                                   \
+			printk_ratelimited(KERN_WARNING fmt, ##__VA_ARGS__);               \
+	} while (0)
+
 #define RAS_DEV_INFO(device, fmt, ...)                                                 \
 	do {                                                                         \
 		if (device)                                                                \
diff --git a/drivers/gpu/drm/amd/ras/rascore/ras_umc.c b/drivers/gpu/drm/amd/ras/rascore/ras_umc.c
index d4072350f48f..e8c13e42c2f8 100644
--- a/drivers/gpu/drm/amd/ras/rascore/ras_umc.c
+++ b/drivers/gpu/drm/amd/ras/rascore/ras_umc.c
@@ -193,12 +193,29 @@ static void ras_umc_reserve_eeprom_record(struct ras_core_context *ras_core,
 }
 
 /* When gpu reset is ongoing, ecc logging operations will be pended.
+ *
+ * The pending list is bounded by RAS_UMC_PENDING_ECC_MAX so that an ECC
+ * storm or repeated UMC error injection cannot make this list (and the
+ * kernel allocations behind it) grow without bound. Once the limit is
+ * reached, additional events are dropped and counted in
+ * pending_ecc_dropped, with a rate-limited warning emitted.
  */
 int ras_umc_log_bad_bank_pending(struct ras_core_context *ras_core, struct ras_bank_ecc *bank)
 {
 	struct ras_umc *ras_umc = &ras_core->ras_umc;
 	struct ras_bank_ecc_node *ecc_node;
 
+	mutex_lock(&ras_umc->pending_ecc_lock);
+	if (ras_umc->pending_ecc_count >= RAS_UMC_PENDING_ECC_MAX) {
+		ras_umc->pending_ecc_dropped++;
+		mutex_unlock(&ras_umc->pending_ecc_lock);
+		RAS_DEV_WARN_RATELIMITED(ras_core->dev,
+			"pending ECC list full (%u), dropping bad bank event (total dropped:%u)\n",
+			RAS_UMC_PENDING_ECC_MAX, ras_umc->pending_ecc_dropped);
+		return -ENOSPC;
+	}
+	mutex_unlock(&ras_umc->pending_ecc_lock);
+
 	ecc_node = kzalloc(sizeof(*ecc_node), GFP_KERNEL);
 	if (!ecc_node)
 		return -ENOMEM;
@@ -206,7 +223,15 @@ int ras_umc_log_bad_bank_pending(struct ras_core_context *ras_core, struct ras_b
 	memcpy(&ecc_node->ecc, bank, sizeof(ecc_node->ecc));
 
 	mutex_lock(&ras_umc->pending_ecc_lock);
+	/* re-check under the lock to honor the cap across concurrent callers */
+	if (ras_umc->pending_ecc_count >= RAS_UMC_PENDING_ECC_MAX) {
+		ras_umc->pending_ecc_dropped++;
+		mutex_unlock(&ras_umc->pending_ecc_lock);
+		kfree(ecc_node);
+		return -ENOSPC;
+	}
 	list_add_tail(&ecc_node->node, &ras_umc->pending_ecc_list);
+	ras_umc->pending_ecc_count++;
 	mutex_unlock(&ras_umc->pending_ecc_lock);
 
 	return 0;
@@ -225,8 +250,16 @@ int ras_umc_log_pending_bad_bank(struct ras_core_context *ras_core)
 		if (!ras_umc_log_bad_bank(ras_core, &ecc_node->ecc)) {
 			list_del(&ecc_node->node);
 			kfree(ecc_node);
+			if (ras_umc->pending_ecc_count)
+				ras_umc->pending_ecc_count--;
 		}
 	}
+	if (ras_umc->pending_ecc_dropped) {
+		RAS_DEV_WARN(ras_core->dev,
+			"%u pending ECC bad-bank events were dropped during GPU reset\n",
+			ras_umc->pending_ecc_dropped);
+		ras_umc->pending_ecc_dropped = 0;
+	}
 	mutex_unlock(&ras_umc->pending_ecc_lock);
 
 	return 0;
@@ -611,6 +644,8 @@ int ras_umc_sw_fini(struct ras_core_context *ras_core)
 		list_del(&ecc_node->node);
 		kfree(ecc_node);
 	}
+	ras_umc->pending_ecc_count = 0;
+	ras_umc->pending_ecc_dropped = 0;
 	mutex_unlock(&ras_umc->pending_ecc_lock);
 
 	mutex_destroy(&ras_umc->tree_lock);
diff --git a/drivers/gpu/drm/amd/ras/rascore/ras_umc.h b/drivers/gpu/drm/amd/ras/rascore/ras_umc.h
index 1d3026be509b..237525b46b9b 100644
--- a/drivers/gpu/drm/amd/ras/rascore/ras_umc.h
+++ b/drivers/gpu/drm/amd/ras/rascore/ras_umc.h
@@ -139,8 +139,20 @@ struct ras_umc {
 	struct mutex  pending_ecc_lock;
 	struct ras_umc_err_data umc_err_data;
 	struct list_head pending_ecc_list;
+	/* number of entries currently queued on pending_ecc_list */
+	u32 pending_ecc_count;
+	/* number of entries dropped because pending_ecc_list was full */
+	u32 pending_ecc_dropped;
 };
 
+/*
+ * Upper bound on entries that can be queued on pending_ecc_list while a
+ * GPU reset is in progress. Beyond this, new ECC events are dropped to
+ * prevent unbounded kernel memory growth in case of an ECC storm or
+ * malicious/repeated UMC error injection.
+ */
+#define RAS_UMC_PENDING_ECC_MAX  8192
+
 int ras_umc_sw_init(struct ras_core_context *ras);
 int ras_umc_sw_fini(struct ras_core_context *ras);
 int ras_umc_hw_init(struct ras_core_context *ras);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/5] drm/amd/ras: snapshot remote cmd header to fix double-fetch
  2026-05-18  9:40 [PATCH 1/5] drm/amdgpu: fix potential overflow in fs_info.debugfs_name Stanley.Yang
  2026-05-18  9:40 ` [PATCH 2/5] drm/amdgpu: init locals in umc_v12_0_convert_error_address Stanley.Yang
  2026-05-18  9:40 ` [PATCH 3/5] drm/amd/ras: cap pending_ecc_list size Stanley.Yang
@ 2026-05-18  9:40 ` Stanley.Yang
  2026-05-18  9:40 ` [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded helpers Stanley.Yang
  3 siblings, 0 replies; 6+ messages in thread
From: Stanley.Yang @ 2026-05-18  9:40 UTC (permalink / raw)
  To: amd-gfx; +Cc: Hawking.Zhang, Tao.Zhou1, YiPeng.Chai, Candice.Li, Stanley.Yang

The response header lives in PF-controlled shared memory. Copy it
into a local struct once, then read cmd_res and output_size from the
snapshot so the PF cannot flip cmd_res or grow output_size between
checks.

Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
---
 .../drm/amd/ras/ras_mgr/amdgpu_virt_ras_cmd.c | 29 +++++++++++++++----
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_virt_ras_cmd.c b/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_virt_ras_cmd.c
index 838eb91aef39..ebbf92a2bd94 100644
--- a/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_virt_ras_cmd.c
+++ b/drivers/gpu/drm/amd/ras/ras_mgr/amdgpu_virt_ras_cmd.c
@@ -92,6 +92,7 @@ static int amdgpu_virt_ras_remote_ioctl_cmd(struct ras_core_context *ras_core,
 	struct amdgpu_virt_ras_cmd *virt_ras = ras_mgr->virt_ras_cmd;
 	uint32_t mem_len = ALIGN(sizeof(*cmd) + output_size, AMDGPU_GPU_PAGE_SIZE);
 	struct ras_cmd_ctx *rcmd;
+	struct ras_cmd_ctx hdr_snap;
 	struct amdgpu_virt_shared_mem shared_mem = {0};
 	int ret = 0;
 
@@ -108,15 +109,31 @@ static int amdgpu_virt_ras_remote_ioctl_cmd(struct ras_core_context *ras_core,
 	ret = amdgpu_virt_send_remote_ras_cmd(ras_core->dev,
 				shared_mem.gpa, mem_len);
 	if (!ret) {
-		if (rcmd->cmd_res) {
-			ret = rcmd->cmd_res;
+		/*
+		 * rcmd lives in shared memory the PF can mutate at any time.
+		 * Snapshot the entire fixed-size response header into a local
+		 * struct in one shot so every subsequent decision (cmd_res,
+		 * output_size, version, etc.) operates on a stable copy. This
+		 * defeats double-fetch / TOCTOU attacks where a malicious or
+		 * buggy PF could flip cmd_res from SUCCESS to an error after
+		 * our success branch, or enlarge output_size between the
+		 * bounds check and the memcpy below to corrupt the caller's
+		 * local output buffer.
+		 */
+		memcpy(&hdr_snap, rcmd, sizeof(hdr_snap));
+		barrier();
+
+		if (hdr_snap.cmd_res) {
+			ret = hdr_snap.cmd_res;
 			goto out;
 		}
 
-		cmd->cmd_res = rcmd->cmd_res;
-		cmd->output_size = rcmd->output_size;
-		if (rcmd->output_size && (rcmd->output_size <= output_size) && output_data)
-			memcpy(output_data, rcmd->output_buff_raw, rcmd->output_size);
+		cmd->cmd_res = hdr_snap.cmd_res;
+		cmd->output_size = hdr_snap.output_size;
+
+		if (hdr_snap.output_size && output_data &&
+		    hdr_snap.output_size <= output_size)
+			memcpy(output_data, rcmd->output_buff_raw, hdr_snap.output_size);
 	}
 
 out:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded helpers
  2026-05-18  9:40 [PATCH 1/5] drm/amdgpu: fix potential overflow in fs_info.debugfs_name Stanley.Yang
                   ` (2 preceding siblings ...)
  2026-05-18  9:40 ` [PATCH 4/5] drm/amd/ras: snapshot remote cmd header to fix double-fetch Stanley.Yang
@ 2026-05-18  9:40 ` Stanley.Yang
  2026-05-19  7:00   ` Zhou1, Tao
  3 siblings, 1 reply; 6+ messages in thread
From: Stanley.Yang @ 2026-05-18  9:40 UTC (permalink / raw)
  To: amd-gfx; +Cc: Hawking.Zhang, Tao.Zhou1, YiPeng.Chai, Candice.Li, Stanley.Yang

Replace the open-coded TLV walk with fru_pia_advance()
and fru_pia_copy_field() helpers that bound every read
by the actual EEPROM data length, preventing out-of-bounds
reads on truncated or malformed FRU data.

Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c    | 95 ++++++++++++-------
 1 file changed, 63 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
index c5178e2b794d..86b2d5a79993 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
@@ -115,6 +115,43 @@ static bool is_fru_eeprom_supported(struct amdgpu_device *adev, u32 *fru_addr)
 	}
 }
 
+/*
+ * IPMI FRU Product Info Area fields are TLV: one type/length byte
+ * (low 6 bits = data length) followed by that many data bytes. These
+ * helpers walk the cursor and copy a single field while bounding all
+ * accesses to the actual buffer length read from the EEPROM.
+ */
+#define FRU_FIELD_LEN(p, a)	((p)[a] & 0x3F)
+
+/* Advance cursor past the current TLV. Returns false if no more data. */
+static bool fru_pia_advance(u32 *addr, const unsigned char *pia, int len)
+{
+	if (*addr >= (u32)len)
+		return false;
+	*addr += 1 + FRU_FIELD_LEN(pia, *addr);
+	return true;
+}
+
+/*
+ * Copy the current TLV's data into dst (NUL-terminated). Returns false if
+ * the TLV header or data would read past the end of pia.
+ */
+static bool fru_pia_copy_field(char *dst, size_t dst_size,
+			       const unsigned char *pia, u32 addr, int len)
+{
+	size_t fl;
+
+	if (addr + 1 >= (u32)len)
+		return false;
+
+	fl = min3((size_t)FRU_FIELD_LEN(pia, addr),
+			  dst_size -1,
+			  (size_t)(len - addr - 1));
+	memcpy(dst, pia + addr + 1, fl);
+	dst[fl] = '\0';
+	return true;
+}
+
 int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
 {
 	struct amdgpu_fru_info *fru_info;
@@ -223,52 +260,46 @@ int amdgpu_fru_get_product_info(struct amdgpu_device *adev)
 	 * Read Manufacturer Name field whose length is [3].
 	 */
 	addr = 3;
-	if (addr + 1 >= len)
+	if (!fru_pia_copy_field(fru_info->manufacturer_name,
+				sizeof(fru_info->manufacturer_name),
+				pia, addr, len))
 		goto Out;
-	memcpy(fru_info->manufacturer_name, pia + addr + 1,
-	       min_t(size_t, sizeof(fru_info->manufacturer_name),
-		     pia[addr] & 0x3F));
-	fru_info->manufacturer_name[sizeof(fru_info->manufacturer_name) - 1] =
-		'\0';
 
 	/* Read Product Name field. */
-	addr += 1 + (pia[addr] & 0x3F);
-	if (addr + 1 >= len)
+	if (!fru_pia_advance(&addr, pia, len) ||
+	    !fru_pia_copy_field(fru_info->product_name,
+				sizeof(fru_info->product_name),
+				pia, addr, len))
 		goto Out;
-	memcpy(fru_info->product_name, pia + addr + 1,
-	       min_t(size_t, sizeof(fru_info->product_name), pia[addr] & 0x3F));
-	fru_info->product_name[sizeof(fru_info->product_name) - 1] = '\0';
 
 	/* Go to the Product Part/Model Number field. */
-	addr += 1 + (pia[addr] & 0x3F);
-	if (addr + 1 >= len)
+	if (!fru_pia_advance(&addr, pia, len) ||
+	    !fru_pia_copy_field(fru_info->product_number,
+				sizeof(fru_info->product_number),
+				pia, addr, len))
 		goto Out;
-	memcpy(fru_info->product_number, pia + addr + 1,
-	       min_t(size_t, sizeof(fru_info->product_number),
-		     pia[addr] & 0x3F));
-	fru_info->product_number[sizeof(fru_info->product_number) - 1] = '\0';
 
-	/* Go to the Product Version field. */
-	addr += 1 + (pia[addr] & 0x3F);
+	/* Skip the Product Version field. */
+	if (!fru_pia_advance(&addr, pia, len))
+		goto Out;
 
-	/* Go to the Product Serial Number field. */
-	addr += 1 + (pia[addr] & 0x3F);
-	if (addr + 1 >= len)
+	/* Read the Product Serial Number field. */
+	if (!fru_pia_advance(&addr, pia, len) ||
+	    !fru_pia_copy_field(fru_info->serial,
+				sizeof(fru_info->serial),
+				pia, addr, len))
 		goto Out;
-	memcpy(fru_info->serial, pia + addr + 1,
-	       min_t(size_t, sizeof(fru_info->serial), pia[addr] & 0x3F));
-	fru_info->serial[sizeof(fru_info->serial) - 1] = '\0';
 
-	/* Asset Tag field */
-	addr += 1 + (pia[addr] & 0x3F);
+	/* Skip the Asset Tag field. */
+	if (!fru_pia_advance(&addr, pia, len))
+		goto Out;
 
 	/* FRU File Id field. This could be 'null'. */
-	addr += 1 + (pia[addr] & 0x3F);
-	if ((addr + 1 >= len) || !(pia[addr] & 0x3F))
+	if (!fru_pia_advance(&addr, pia, len) ||
+	    !fru_pia_copy_field(fru_info->fru_id,
+				sizeof(fru_info->fru_id),
+				pia, addr, len))
 		goto Out;
-	memcpy(fru_info->fru_id, pia + addr + 1,
-	       min_t(size_t, sizeof(fru_info->fru_id), pia[addr] & 0x3F));
-	fru_info->fru_id[sizeof(fru_info->fru_id) - 1] = '\0';
 
 Out:
 	kfree(pia);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded helpers
  2026-05-18  9:40 ` [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded helpers Stanley.Yang
@ 2026-05-19  7:00   ` Zhou1, Tao
  0 siblings, 0 replies; 6+ messages in thread
From: Zhou1, Tao @ 2026-05-19  7:00 UTC (permalink / raw)
  To: Yang, Stanley, amd-gfx@lists.freedesktop.org
  Cc: Zhang, Hawking, Chai, Thomas, Li, Candice

AMD General

The series is: Reviewed-by: Tao Zhou <tao.zhou1@amd.com>

> -----Original Message-----
> From: Yang, Stanley <Stanley.Yang@amd.com>
> Sent: Monday, May 18, 2026 5:40 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Zhou1, Tao
> <Tao.Zhou1@amd.com>; Chai, Thomas <YiPeng.Chai@amd.com>; Li, Candice
> <Candice.Li@amd.com>; Yang, Stanley <Stanley.Yang@amd.com>
> Subject: [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded
> helpers
>
> Replace the open-coded TLV walk with fru_pia_advance() and
> fru_pia_copy_field() helpers that bound every read by the actual EEPROM
> data length, preventing out-of-bounds reads on truncated or malformed FRU
> data.
>
> Signed-off-by: Stanley.Yang <Stanley.Yang@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c    | 95 ++++++++++++-------
>  1 file changed, 63 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index c5178e2b794d..86b2d5a79993 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -115,6 +115,43 @@ static bool is_fru_eeprom_supported(struct
> amdgpu_device *adev, u32 *fru_addr)
>       }
>  }
>
> +/*
> + * IPMI FRU Product Info Area fields are TLV: one type/length byte
> + * (low 6 bits = data length) followed by that many data bytes. These
> + * helpers walk the cursor and copy a single field while bounding all
> + * accesses to the actual buffer length read from the EEPROM.
> + */
> +#define FRU_FIELD_LEN(p, a)  ((p)[a] & 0x3F)
> +
> +/* Advance cursor past the current TLV. Returns false if no more data.
> +*/ static bool fru_pia_advance(u32 *addr, const unsigned char *pia, int
> +len) {
> +     if (*addr >= (u32)len)
> +             return false;
> +     *addr += 1 + FRU_FIELD_LEN(pia, *addr);
> +     return true;
> +}
> +
> +/*
> + * Copy the current TLV's data into dst (NUL-terminated). Returns false
> +if
> + * the TLV header or data would read past the end of pia.
> + */
> +static bool fru_pia_copy_field(char *dst, size_t dst_size,
> +                            const unsigned char *pia, u32 addr, int len) {
> +     size_t fl;
> +
> +     if (addr + 1 >= (u32)len)
> +             return false;
> +
> +     fl = min3((size_t)FRU_FIELD_LEN(pia, addr),
> +                       dst_size -1,
> +                       (size_t)(len - addr - 1));
> +     memcpy(dst, pia + addr + 1, fl);
> +     dst[fl] = '\0';
> +     return true;
> +}
> +
>  int amdgpu_fru_get_product_info(struct amdgpu_device *adev)  {
>       struct amdgpu_fru_info *fru_info;
> @@ -223,52 +260,46 @@ int amdgpu_fru_get_product_info(struct
> amdgpu_device *adev)
>        * Read Manufacturer Name field whose length is [3].
>        */
>       addr = 3;
> -     if (addr + 1 >= len)
> +     if (!fru_pia_copy_field(fru_info->manufacturer_name,
> +                             sizeof(fru_info->manufacturer_name),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->manufacturer_name, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->manufacturer_name),
> -                  pia[addr] & 0x3F));
> -     fru_info->manufacturer_name[sizeof(fru_info->manufacturer_name)
> - 1] =
> -             '\0';
>
>       /* Read Product Name field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if (addr + 1 >= len)
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->product_name,
> +                             sizeof(fru_info->product_name),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->product_name, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->product_name), pia[addr] & 0x3F));
> -     fru_info->product_name[sizeof(fru_info->product_name) - 1] = '\0';
>
>       /* Go to the Product Part/Model Number field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if (addr + 1 >= len)
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->product_number,
> +                             sizeof(fru_info->product_number),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->product_number, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->product_number),
> -                  pia[addr] & 0x3F));
> -     fru_info->product_number[sizeof(fru_info->product_number) - 1] =
> '\0';
>
> -     /* Go to the Product Version field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> +     /* Skip the Product Version field. */
> +     if (!fru_pia_advance(&addr, pia, len))
> +             goto Out;
>
> -     /* Go to the Product Serial Number field. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if (addr + 1 >= len)
> +     /* Read the Product Serial Number field. */
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->serial,
> +                             sizeof(fru_info->serial),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->serial, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->serial), pia[addr] & 0x3F));
> -     fru_info->serial[sizeof(fru_info->serial) - 1] = '\0';
>
> -     /* Asset Tag field */
> -     addr += 1 + (pia[addr] & 0x3F);
> +     /* Skip the Asset Tag field. */
> +     if (!fru_pia_advance(&addr, pia, len))
> +             goto Out;
>
>       /* FRU File Id field. This could be 'null'. */
> -     addr += 1 + (pia[addr] & 0x3F);
> -     if ((addr + 1 >= len) || !(pia[addr] & 0x3F))
> +     if (!fru_pia_advance(&addr, pia, len) ||
> +         !fru_pia_copy_field(fru_info->fru_id,
> +                             sizeof(fru_info->fru_id),
> +                             pia, addr, len))
>               goto Out;
> -     memcpy(fru_info->fru_id, pia + addr + 1,
> -            min_t(size_t, sizeof(fru_info->fru_id), pia[addr] & 0x3F));
> -     fru_info->fru_id[sizeof(fru_info->fru_id) - 1] = '\0';
>
>  Out:
>       kfree(pia);
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-19  7:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18  9:40 [PATCH 1/5] drm/amdgpu: fix potential overflow in fs_info.debugfs_name Stanley.Yang
2026-05-18  9:40 ` [PATCH 2/5] drm/amdgpu: init locals in umc_v12_0_convert_error_address Stanley.Yang
2026-05-18  9:40 ` [PATCH 3/5] drm/amd/ras: cap pending_ecc_list size Stanley.Yang
2026-05-18  9:40 ` [PATCH 4/5] drm/amd/ras: snapshot remote cmd header to fix double-fetch Stanley.Yang
2026-05-18  9:40 ` [PATCH 5/5] drm/amdgpu: harden FRU PIA parsing with bounded helpers Stanley.Yang
2026-05-19  7:00   ` Zhou1, Tao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox