[PATCH] iscsi_ibft: validate iBFT string fields against table boundary

[Junrui Luo]

The ibft_attr_show_initiator(), ibft_attr_show_nic(), and
ibft_attr_show_target() functions construct pointers into the iBFT table
using firmware-supplied offset fields:

    (char *)ibft_loc + field_off

Neither the offset nor the accompanying length field is checked against
the table length, so a crafted iBFT table can direct these reads outside
the table boundary.

On the ISA path, the table resides in the legacy 512 kB–1 MB physical
region and the u16 offset field cannot reach beyond that region, so the
out-of-bounds read is limited to adjacent firmware-reserved memory.  On
the ACPI path the table is mapped for exactly header.length bytes, so
reading past the boundary accesses unmapped virtual memory, which causes
a kernel panic.

Introduce ibft_str_ptr(), which verifies that off + len does not exceed
header->header.length before returning the pointer.  Make sprintf_string()
return 0 on NULL so callers require no additional guard.

Additionally, ibft_check_device() re-reads header.length from firmware
memory on the ISA path without applying the bound that
reserve_ibft_region() enforced at scan time.  Restore that check so a
modified header.length cannot cause the checksum loop itself to read
outside the reserved physical region.

Fixes: 138fe4e06979 ("Firmware: add iSCSI iBFT Support")
Reported-by: Yuhao Jiang <danisjiang@xxxxxxxxx>
Signed-off-by: Junrui Luo <moonafterrain@xxxxxxxxxxx>
---
 drivers/firmware/iscsi_ibft.c | 62 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
index e457b6049e83..9709358436ba 100644
--- a/drivers/firmware/iscsi_ibft.c
+++ b/drivers/firmware/iscsi_ibft.c
@@ -220,9 +220,23 @@ static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
 
 static ssize_t sprintf_string(char *str, int len, char *buf)
 {
+	if (!buf)
+		return 0;
 	return sprintf(str, "%.*s\n", len, buf);
 }
 
+/*
+ * Validate that a string field described by (off, len) lies entirely within
+ * the iBFT table, then return a pointer to it.  Returns NULL if the field
+ * is absent (off == 0 or len == 0) or would read past the table boundary.
+ */
+static char *ibft_str_ptr(struct acpi_table_ibft *header, u16 off, u16 len)
+{
+	if (!off || !len || (u32)off + len > header->header.length)
+		return NULL;
+	return (char *)header + off;
+}
+
 /*
  * Helper function to verify the IBFT header.
  */
@@ -251,7 +265,6 @@ static ssize_t ibft_attr_show_initiator(void *data, int type, char *buf)
 {
 	struct ibft_kobject *entry = data;
 	struct ibft_initiator *initiator = entry->initiator;
-	void *ibft_loc = entry->header;
 	char *str = buf;
 
 	if (!initiator)
@@ -278,8 +291,9 @@ static ssize_t ibft_attr_show_initiator(void *data, int type, char *buf)
 		break;
 	case ISCSI_BOOT_INI_INITIATOR_NAME:
 		str += sprintf_string(str, initiator->initiator_name_len,
-				      (char *)ibft_loc +
-				      initiator->initiator_name_off);
+				      ibft_str_ptr(entry->header,
+						   initiator->initiator_name_off,
+						   initiator->initiator_name_len));
 		break;
 	default:
 		break;
@@ -292,7 +306,6 @@ static ssize_t ibft_attr_show_nic(void *data, int type, char *buf)
 {
 	struct ibft_kobject *entry = data;
 	struct ibft_nic *nic = entry->nic;
-	void *ibft_loc = entry->header;
 	char *str = buf;
 	__be32 val;
 
@@ -342,7 +355,9 @@ static ssize_t ibft_attr_show_nic(void *data, int type, char *buf)
 		break;
 	case ISCSI_BOOT_ETH_HOSTNAME:
 		str += sprintf_string(str, nic->hostname_len,
-				      (char *)ibft_loc + nic->hostname_off);
+				      ibft_str_ptr(entry->header,
+						   nic->hostname_off,
+						   nic->hostname_len));
 		break;
 	default:
 		break;
@@ -355,7 +370,6 @@ static ssize_t ibft_attr_show_target(void *data, int type, char *buf)
 {
 	struct ibft_kobject *entry = data;
 	struct ibft_tgt *tgt = entry->tgt;
-	void *ibft_loc = entry->header;
 	char *str = buf;
 	int i;
 
@@ -388,25 +402,33 @@ static ssize_t ibft_attr_show_target(void *data, int type, char *buf)
 		break;
 	case ISCSI_BOOT_TGT_NAME:
 		str += sprintf_string(str, tgt->tgt_name_len,
-				      (char *)ibft_loc + tgt->tgt_name_off);
+				      ibft_str_ptr(entry->header,
+						   tgt->tgt_name_off,
+						   tgt->tgt_name_len));
 		break;
 	case ISCSI_BOOT_TGT_CHAP_NAME:
 		str += sprintf_string(str, tgt->chap_name_len,
-				      (char *)ibft_loc + tgt->chap_name_off);
+				      ibft_str_ptr(entry->header,
+						   tgt->chap_name_off,
+						   tgt->chap_name_len));
 		break;
 	case ISCSI_BOOT_TGT_CHAP_SECRET:
 		str += sprintf_string(str, tgt->chap_secret_len,
-				      (char *)ibft_loc + tgt->chap_secret_off);
+				      ibft_str_ptr(entry->header,
+						   tgt->chap_secret_off,
+						   tgt->chap_secret_len));
 		break;
 	case ISCSI_BOOT_TGT_REV_CHAP_NAME:
 		str += sprintf_string(str, tgt->rev_chap_name_len,
-				      (char *)ibft_loc +
-				      tgt->rev_chap_name_off);
+				      ibft_str_ptr(entry->header,
+						   tgt->rev_chap_name_off,
+						   tgt->rev_chap_name_len));
 		break;
 	case ISCSI_BOOT_TGT_REV_CHAP_SECRET:
 		str += sprintf_string(str, tgt->rev_chap_secret_len,
-				      (char *)ibft_loc +
-				      tgt->rev_chap_secret_off);
+				      ibft_str_ptr(entry->header,
+						   tgt->rev_chap_secret_off,
+						   tgt->rev_chap_secret_len));
 		break;
 	default:
 		break;
@@ -448,6 +470,20 @@ static int __init ibft_check_device(void)
 
 	len = ibft_addr->header.length;
 
+#ifdef CONFIG_ISCSI_IBFT_FIND
+	/*
+	 * For the ISA path, header.length is re-read from physical memory
+	 * that memblock_reserve() does not write-protect.  Re-validate the
+	 * same physical bound that reserve_ibft_region() checked at scan
+	 * time: the table must fit entirely within [ibft_phys_addr, IBFT_END).
+	 */
+	if (ibft_phys_addr &&
+	    (len < sizeof(*ibft_addr) || len > IBFT_END - ibft_phys_addr)) {
+		printk(KERN_ERR "iBFT error: table length %d out of valid ISA range.\n", len);
+		return -ENOENT;
+	}
+#endif
+
 	/* Sanity checking of iBFT. */
 	if (ibft_addr->header.revision != 1) {
 		printk(KERN_ERR "iBFT module supports only revision 1, " \

---
base-commit: 0257f64bdac7fdca30fa3cae0df8b9ecbec7733a
change-id: 20260313-fixes-a2db6f1b2c70

Best regards,
-- 
Junrui Luo <moonafterrain@xxxxxxxxxxx>