From: Konrad Rzeszutek <konradr@redhat.com>
Date: Wed, 20 Feb 2008 13:20:22 -0500
Subject: [firmware] ibft_iscsi: prevent misconfigured iBFTs
Message-id: 20080220182022.GA3331@mars.boston.redhat.com
O-Subject: Re: [RHEL5 PATCH] RHBZ#430297 - ibft_iscsi when used with Intel NIC which have iBFT misconfigured crashes.
Bugzilla: 430297

> > > On Fri, Jan 25, 2008 at 05:11:52PM -0500, Konrad Rzeszutek wrote:
> > > > RHBZ#:
> > > > ------
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=430297
> > > >
> > > > Description:
> > > > ------------
> > > > The Intel NICs which support iSCSI can write a completely bogus iBFT structure.
> > > > The iscsi_ibft doesn't handle that gracefully and this can result in a panic.
> > > > The work-around is to configure the Intel NIC properly - which might not be
> > > > obvious to causal user or this patch.
> > > >
> > > > Attached ia a patch to RHEL5 which makes the module more robust.
> > > >
> > > > Upstream Status:
> > > > ----------------
> > > > http://lkml.org/lkml/2008/1/25/549
> > >

Acked-by: Mike Christie <mchristi@redhat.com>

diff --git a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c
index 4f74495..34b515d 100644
--- a/arch/i386/kernel/setup.c
+++ b/arch/i386/kernel/setup.c
@@ -721,20 +721,6 @@ static inline void copy_edd(void)
 }
 #endif
 
-#if defined(CONFIG_ISCSI_IBFT_FIND)
-static void __init reserve_ibft_region(void)
-{
-	unsigned int ibft_len;
-
-	ibft_len = find_ibft();
-	if (ibft_len)
-		reserve_bootmem((unsigned int)ibft_phys,
-				PAGE_ALIGN(ibft_len));
-}
-#else
-static void __init reserve_ibft_region(void) { }
-#endif
-
 static void __init parse_cmdline_early (char ** cmdline_p)
 {
 	char c = ' ', *to = command_line, *from = saved_command_line;
diff --git a/arch/x86_64/kernel/setup.c b/arch/x86_64/kernel/setup.c
index 3278347..19ca938 100644
--- a/arch/x86_64/kernel/setup.c
+++ b/arch/x86_64/kernel/setup.c
@@ -494,20 +494,6 @@ static inline void copy_edd(void)
 }
 #endif
 
-#if defined(CONFIG_ISCSI_IBFT_FIND)
-static void __init reserve_ibft_region(void)
-{
-	unsigned int ibft_len;
-
-	ibft_len = find_ibft();
-	if (ibft_len)
-		reserve_bootmem_generic((unsigned long)ibft_phys,
-					PAGE_ALIGN(ibft_len));
-}
-#else
-static void __init reserve_ibft_region(void) { }
-#endif
-
 #define EBDA_ADDR_POINTER 0x40E
 
 unsigned __initdata ebda_addr;
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 9f1b54e..9a0acfa 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -86,6 +86,7 @@ config DCDBAS
 
 config ISCSI_IBFT_FIND
 	bool "iSCSI Boot Firmware Table Attributes"
+	depends on X86
 	default n
 	help
 	  This option enables the kernel to find the region of memory
diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
index bbd41f1..4c76064 100644
--- a/drivers/firmware/iscsi_ibft.c
+++ b/drivers/firmware/iscsi_ibft.c
@@ -1,7 +1,7 @@
 /*
  *  Copyright 2007 Red Hat, Inc.
  *  by Peter Jones <pjones@redhat.com>
- *  Copyright 2007 IBM, Inc.
+ *  Copyright 2008 IBM, Inc.
  *  by Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
  *
  * This code exposes the iSCSI Boot Format Table to userland via sysfs.
@@ -14,6 +14,48 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
+ *
+ * Changelog:
+ *
+ *  11 Feb 2008 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *    Converted to using ibft_addr. (v0.4.8)
+ *
+ *   8 Feb 2008 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *    Combined two functions in one: reserve_ibft_region. (v0.4.7)
+ *
+ *  30 Jan 2008 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added logic to handle IPv6 addresses. (v0.4.6)
+ *
+ *  25 Jan 2008 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added logic to handle badly not-to-spec iBFT. (v0.4.5)
+ *
+ *   4 Jan 2008 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added __init to function declarations. (v0.4.4)
+ *
+ *  21 Dec 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Updated kobject registration, combined unregister functions in one
+ *   and code and style cleanup. (v0.4.3)
+ *
+ *   5 Dec 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added end-markers to enums and re-organized kobject registration. (v0.4.2)
+ *
+ *   4 Dec 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Created 'device' sysfs link to the NIC and style cleanup. (v0.4.1)
+ *
+ *  28 Nov 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added sysfs-ibft documentation, moved 'find_ibft' function to
+ *   in its own file and added text attributes for every struct field.  (v0.4)
+ *
+ *  21 Nov 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added text attributes emulating OpenFirmware /proc/device-tree naming.
+ *   Removed binary /sysfs interface (v0.3)
+ *
+ *  29 Aug 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   Added functionality in setup.c to reserve iBFT region. (v0.2)
+ *
+ *  27 Aug 2007 - Konrad Rzeszutek <konradr@linux.vnet.ibm.com>
+ *   First version exposing iBFT data via a binary /sysfs. (v0.1)
+ *
  */
 
 
@@ -31,8 +73,8 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
-#define IBFT_ISCSI_VERSION "0.4.4-rhel5"
-#define IBFT_ISCSI_DATE "2008-Jan-9"
+#define IBFT_ISCSI_VERSION "0.4.8-rhel5"
+#define IBFT_ISCSI_DATE "2008-Feb-11"
 
 MODULE_AUTHOR("Peter Jones <pjones@redhat.com> and \
 Konrad Rzeszutek <konradr@us.ibm.com>");
@@ -40,17 +82,6 @@ MODULE_DESCRIPTION("sysfs interface to BIOS iBFT information");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(IBFT_ISCSI_VERSION);
 
-
-struct ibft_table_header {
-	char signature[4];
-	u32 length;
-	u8 revision;
-	u8 checksum;
-	char oem_id[6];
-	char oem_table_id[8];
-	char reserved[24];
-} __attribute__((__packed__));
-
 struct ibft_hdr {
 	u8 id;
 	u8 version;
@@ -119,17 +150,20 @@ struct ibft_tgt {
  *
 */
 enum ibft_id {
-	id_reserved = 0, /* Should never show up */
-	id_control = 1, /* Ditto */
+	id_reserved = 0, /* We don't support. */
+	id_control = 1, /* Should show up only once and is not exported. */
 	id_initiator = 2,
 	id_nic = 3,
 	id_target = 4,
-	id_extensions, /* We don't support. */
+	id_extensions = 5, /* We don't support. */
+	id_end_marker,
 };
 
+/*
+ * We do not support the other types, hence the NULL.
+ */
 static const char *ibft_id_names[] =
-	{"reserved", "control", "initiator", "ethernet%d",
-	"target%d", "extension%d", NULL};
+	{NULL, NULL, "initiator", "ethernet%d", "target%d", NULL, NULL};
 
 /*
  * The text attributes names for each of the kobjects.
@@ -229,23 +263,32 @@ static LIST_HEAD(ibft_attr_list);
 static LIST_HEAD(ibft_kobject_list);
 
 static const char nulls[16];
-static struct ibft_table_header *ibft_device;
 
 /*
  * Helper functions to parse data properly.
  */
 static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
 {
+	char *str = buf;
+
 	if (ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 &&
 	    ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && ip[7] == 0 &&
 	    ip[8] == 0 && ip[9] == 0 && ip[10] == 0xff && ip[11] == 0xff) {
 		/*
 		 * IPV4
 		 */
-		return sprintf(buf, "%d.%d.%d.%d\n", ip[12],
+		str += sprintf(buf, NIPQUAD_FMT, ip[12],
 			       ip[13], ip[14], ip[15]);
-	} else
-		return 0;
+	} else {
+		/*
+		 * IPv6
+		 */
+		str += sprintf(str, NIP6_FMT, ntohs(ip[0]), ntohs(ip[1]),
+			       ntohs(ip[2]), ntohs(ip[3]), ntohs(ip[4]),
+			       ntohs(ip[5]), ntohs(ip[6]), ntohs(ip[7]));
+	}
+	str += sprintf(str, "\n");
+	return str - buf;
 }
 
 static ssize_t sprintf_string(char *str, int len, char *buf)
@@ -256,18 +299,21 @@ static ssize_t sprintf_string(char *str, int len, char *buf)
 /*
  * Helper function to verify the IBFT header.
  */
-static int ibft_verify_hdr(struct ibft_hdr *hdr, int id, int length)
+static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int length)
 {
-#define IBFT_VERIFY_HDR_FIELD(val, name) \
-	if (hdr->val != val) { \
-		printk(KERN_ERR \
-		       "error, in IBFT structure (%s) expected %d but" \
-		       " found %d\n", \
-		       name, val, hdr->val); \
-		return -ENODEV; \
+	if (hdr->id != id) {
+		printk(KERN_ERR "iBFT error: The field header.id is " \
+				"expected %d but found %d!\n", \
+				id, hdr->id);
+		return -ENODEV;
+	}
+	if (hdr->length != length) {
+		printk(KERN_ERR "iBFT error: The field header.length is " \
+				"expected %d but found %d!\n", \
+				length, hdr->length);
+		return -ENODEV;
 	}
-	IBFT_VERIFY_HDR_FIELD(id, "ID");
-	IBFT_VERIFY_HDR_FIELD(length, "Length");
+
 	return 0;
 }
 
@@ -486,32 +532,31 @@ static struct kobj_type ibft_ktype = {
 
 static decl_subsys(ibft, &ibft_ktype, NULL);
 
-static int __init ibft_alloc_device(void *idev)
+static int __init ibft_check_device(void)
 {
 	int len;
-	struct ibft_table_header *hdr;
-
-	if (!idev)
-		return -ENOENT;
+	u8 *pos;
+	u8 csum = 0;
 
-	hdr = (struct ibft_table_header *)phys_to_virt((unsigned long)
-						       ibft_phys);
-	len = hdr->length;
+	len = ibft_addr->length;
 
-	ibft_device = kmalloc(len, GFP_KERNEL);
-	if (!ibft_device)
-		return -ENOMEM;
+	/* Sanity checking of iBFT. */
+	if (ibft_addr->revision != 1) {
+		printk(KERN_ERR "iBFT module supports only revision 1, " \
+				"while this is %d.\n", ibft_addr->revision);
+		return 1;
+	}
+	for (pos = (u8 *)ibft_addr; pos < (u8 *)ibft_addr + len; pos++)
+		csum += *pos;
 
-	memcpy(ibft_device, hdr, len);
+	if (csum) {
+		printk(KERN_ERR "iBFT has incorrect checksum (0x%x)!\n", csum);
+		return 1;
+	}
 
 	return 0;
 }
 
-static void ibft_free_device(struct ibft_table_header *hdr)
-{
-	kfree(hdr);
-}
-
 /*
  * Helper function for ibft_register_kobjects.
  */
@@ -533,25 +578,33 @@ static int __init ibft_create_kobject(struct ibft_table_header *header,
 
 	switch (hdr->id) {
 	case id_initiator:
-		rc = ibft_verify_hdr(hdr, id_initiator,
+		rc = ibft_verify_hdr("initiator", hdr, id_initiator,
 				     sizeof(*ibft_kobj->initiator));
 		break;
 	case id_nic:
-		rc = ibft_verify_hdr(hdr, id_nic,
+		rc = ibft_verify_hdr("ethernet", hdr, id_nic,
 				     sizeof(*ibft_kobj->nic));
 		break;
 	case id_target:
-		rc = ibft_verify_hdr(hdr, id_target,
+		rc = ibft_verify_hdr("target", hdr, id_target,
 				     sizeof(*ibft_kobj->tgt));
 		break;
+	case id_reserved:
+	case id_control:
+	case id_extensions:
+		/* Fields which we don't support. Ignore them */
+		rc = 1;
+		break;
 	default:
-		/* Extension field which we don't support. Ignore it */
+		printk(KERN_ERR "iBFT has unknown structure type (%d). " \
+				"Report this bug to your vendor!\n", hdr->id);
+		rc = 1;
 		break;
 	}
 
 	if (rc) {
 		kfree(ibft_kobj);
-		goto out;
+		goto out_invalid_struct;
 	}
 
 	/* kobject fief. We will let the reference counter do the job
@@ -592,6 +645,9 @@ kobject_do_put:
 	kobject_put(&ibft_kobj->kobj);
 out:
 	return rc;
+out_invalid_struct:
+	/* Unsupported structs are skipped. */
+	return 0;
 }
 
 /*
@@ -605,13 +661,21 @@ static int __init ibft_register_kobjects(struct ibft_table_header *header,
 	void *ptr, *end;
 	int rc = 0;
 	u16 offset;
+	u16 eot_offset;
 
 	control = (void *)header + sizeof(*header);
 	end = (void *)control + control->hdr.length;
+	eot_offset = (void *)header + header->length -
+		     (void *)control - sizeof(*header);
+	rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control,
+			     sizeof(*control));
+	rc |= control->hdr.index != 0;
+	if (rc)
+		return rc;
 
 	for (ptr = &control->initiator_off; ptr < end; ptr += sizeof(u16)) {
 		offset = *(u16 *)ptr;
-		if (offset && offset < header->length) {
+		if (offset && offset < header->length && offset < eot_offset) {
 			rc = ibft_create_kobject(header,
 						 (void *)header + offset,
 						 list);
@@ -863,22 +927,20 @@ static int __init ibft_init(void)
 {
 	int rc = 0;
 
-	if (!ibft_phys)
-		find_ibft();
-
 	rc = firmware_register(&ibft_subsys);
 	if (rc)
 		return rc;
 
-	if (ibft_phys) {
-		printk(KERN_INFO "iBFT detected at 0x%p.\n", ibft_phys);
+	if (ibft_addr) {
+		printk(KERN_INFO "iBFT detected at 0x%lx.\n",
+		       virt_to_phys((void *)ibft_addr));
 
-		rc = ibft_alloc_device(ibft_phys);
+		rc = ibft_check_device();
 		if (rc)
 			goto out_firmware_unregister;
 
 		/* Scan the IBFT for data and register kobjects. */
-		rc = ibft_register_kobjects(ibft_device, &ibft_kobject_list);
+		rc = ibft_register_kobjects(ibft_addr, &ibft_kobject_list);
 		if (rc)
 			goto out_free;
 
@@ -894,7 +956,6 @@ static int __init ibft_init(void)
 
 out_free:
 	ibft_unregister(&ibft_attr_list, &ibft_kobject_list);
-	ibft_free_device(ibft_device);
 out_firmware_unregister:
 	firmware_unregister(&ibft_subsys);
 	return rc;
@@ -903,7 +964,6 @@ out_firmware_unregister:
 static void __exit ibft_exit(void)
 {
 	ibft_unregister(&ibft_attr_list, &ibft_kobject_list);
-	ibft_free_device(ibft_device);
 	firmware_unregister(&ibft_subsys);
 }
 
diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index d40cc25..8ef433a 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -16,6 +16,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/bootmem.h>
 #include <linux/blkdev.h>
 #include <linux/ctype.h>
 #include <linux/device.h>
@@ -29,12 +30,13 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
+#include <asm/mmzone.h>
 
 /*
  * Physical location of iSCSI Boot Format Table.
  */
-void *ibft_phys;
-EXPORT_SYMBOL(ibft_phys);
+struct ibft_table_header *ibft_addr;
+EXPORT_SYMBOL(ibft_addr);
 
 #define IBFT_SIGN "iBFT"
 #define IBFT_SIGN_LEN 4
@@ -45,16 +47,18 @@ EXPORT_SYMBOL(ibft_phys);
 
 
 /*
- * Routine used to find the iSCSI Boot Format Table. the physical
- * location is set in the ibft_phys variable. The return value is
- * the size of IBFT.
+ * Routine used to find the iSCSI Boot Format Table. the logical
+ * kernel address is set in the ibft_addr variable.
  */
-ssize_t find_ibft(void)
+void __init reserve_ibft_region(void)
 {
 	unsigned long pos;
+	unsigned int len = 0;
+	void *virt;
+
+	ibft_addr = 0;
 
 	for (pos = IBFT_START; pos < IBFT_END; pos += 16) {
-		void *virt;
 		/* The table can't be inside the VGA BIOS reserved space,
 		 * so skip that area */
 		if (pos == VGA_MEM)
@@ -63,15 +67,16 @@ ssize_t find_ibft(void)
 		if (memcmp(virt, IBFT_SIGN, IBFT_SIGN_LEN) == 0) {
 			unsigned long *addr =
 			    (unsigned long *)phys_to_virt(pos + 4);
-			unsigned int len = *addr;
+			len = *addr;
 			/* if the length of the table extends past 1M,
 			 * the table cannot be valid. */
 			if (pos + len <= (IBFT_END-1)) {
-				ibft_phys = (void *)pos;
-				return len;
+				ibft_addr = (struct ibft_table_header *)virt;
+				break;
 			}
 		}
 	}
-	return 0;
+	if (ibft_addr)
+		reserve_bootmem(pos, PAGE_ALIGN(len));
 }
-EXPORT_SYMBOL(find_ibft);
+EXPORT_SYMBOL(reserve_ibft_region);
diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h
index d36ca16..6b12659 100644
--- a/include/linux/iscsi_ibft.h
+++ b/include/linux/iscsi_ibft.h
@@ -19,20 +19,30 @@
 #ifndef ISCSI_IBFT_H
 #define ISCSI_IBFT_H
 
-#if defined(CONFIG_ISCSI_IBFT_FIND)
+struct ibft_table_header {
+	char signature[4];
+	u32 length;
+	u8 revision;
+	u8 checksum;
+	char oem_id[6];
+	char oem_table_id[8];
+	char reserved[24];
+} __attribute__((__packed__));
 
 /*
- * Physical location of iSCSI Boot Format Table.
+ * Logical location of iSCSI Boot Format Table.
+ * If the value is NULL there is no iBFT on the machine.
  */
-extern void *ibft_phys;
+extern struct ibft_table_header *ibft_addr;
 
 /*
- * Routine used to find the iSCSI Boot Format Table. The physical
- * location is set in the ibft_phys variable. The return value is the
- * size of IBFT.
+ * Routine used to find and reserve the iSCSI Boot Format Table. The
+ * mapped address is set in the ibft_addr variable.
  */
-extern ssize_t find_ibft(void);
-
+#ifdef CONFIG_ISCSI_IBFT_FIND
+extern void __init reserve_ibft_region(void);
+#else
+static inline void reserve_ibft_region(void) { }
 #endif
 
 #endif /* ISCSI_IBFT_H */