[PATCH v7 3/4] USB: io_ti: Add firmware image sanity checks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Peter E. Berger" <pberger@xxxxxxxxxxx>

Do what we can to verify that the driver's firmware image is valid
(before attempting to download it to the Edgeport) by adding a new
function, check_fw_sanity(), and a call to it in in download_fw().
Also add an fw == NULL check in edge_startup().

Note: It looks like some Edgeports (models like the EP/416 with on-board
E2PROM) may be able to function even if the on-disk firmware image is
bad or missing, iff their local E2PROM versions are valid.  But most
Edgeport models (I've tried EP/1 and EP/8) do not appear to have this
capability and they always rely on the on-disk firmware image.

I tested an implementation that calls the new check_fw_sanity()
function at the top of download_fw() and, rather than simply returning
an error if the firmware image is bad or missing, it saves the result
and defers the decision until later when it may find that it is running
on a E2PROM-equipped device with a valid image.  But I think this is
messier than it is worth (adding still more messiness to the already
very messy download_fw()) for such a marginal possible benefit.  So, at
least for now, I have chosen the much simpler approach of returning an
error whenever edge_startup() fails to load an on-disk firmware image, or
check_fw_sanity() indicates that it is unusable.

Signed-off-by: Peter E. Berger <pberger@xxxxxxxxxxx>
---
 drivers/usb/serial/io_ti.c | 63 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 4492c17..7c5f6fd 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -35,6 +35,7 @@
 #include <linux/uaccess.h>
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
+#include <asm/unaligned.h>
 
 #include "io_16654.h"
 #include "io_usbvend.h"
@@ -909,6 +910,64 @@ static int ti_cpu_rev(struct edge_ti_manuf_descriptor *desc)
 	return TI_GET_CPU_REVISION(desc->CpuRev_BoardRev);
 }
 
+/*
+ * Edgeport firmware images start with a 7-byte header:
+ *
+ *      u8 major_version;
+ *      u8 minor_version;
+ *      le16 build_number;
+ *      le16 length;
+ *      u8 checksum;
+ *
+ * "build_number" has been set to 0 in all three of the images I have
+ * seen, and Digi Tech Support suggests that it is safe to ignore it.
+ *
+ * "length" is the number of bytes of actual data following the header.
+ *
+ * "checksum" is the low order byte resulting from adding the values of
+ * all the data bytes.
+ *
+ */
+static int check_fw_sanity(struct edgeport_serial *serial,
+		const struct firmware *fw)
+{
+#define FW_HEADER_SIZE 7
+#define FW_LENGTH_OFFSET 4
+#define FW_CHECKSUM_OFFSET 6
+	u16 length_data;
+	u16 length_total;
+	u8 checksum;
+	int checksum_new = 0;
+	int pos;
+	struct device *dev = &serial->serial->interface->dev;
+
+	if (fw->size < FW_HEADER_SIZE) {
+		dev_err(dev, "%s - Incomplete fw header\n", __func__);
+		return 1;
+	}
+
+	length_data = get_unaligned_le16(&fw->data[FW_LENGTH_OFFSET]);
+	checksum = fw->data[FW_CHECKSUM_OFFSET];
+	length_total = length_data + FW_HEADER_SIZE;
+
+	if (fw->size != length_total) {
+		dev_err(dev, "%s - Bad fw size (Expected:%d, Got:%d)\n",
+				__func__, length_total, (int)fw->size);
+		return 1;
+	}
+
+	for (pos = FW_HEADER_SIZE; pos < length_total; ++pos)
+		checksum_new = (checksum_new + fw->data[pos]) & 0xFF;
+
+	if (checksum_new != checksum) {
+		dev_err(dev, "%s - Bad fw checksum (Expected:0x%x, Got:0x%x)\n",
+				 __func__, checksum, checksum_new);
+		return 1;
+	}
+
+	return 0;
+}
+
 /**
  * DownloadTIFirmware - Download run-time operating firmware to the TI5052
  *
@@ -928,6 +987,8 @@ static int download_fw(struct edgeport_serial *serial,
 	u8 fw_major_version;
 	u8 fw_minor_version;
 
+	if (check_fw_sanity(serial, fw))
+		return -EINVAL;
 	fw_major_version = fw->data[0];
 	fw_minor_version = fw->data[1];
 	/* If on-board version is newer, "fw_version" will be updated below. */
@@ -2377,7 +2438,7 @@ static int edge_startup(struct usb_serial *serial)
 	usb_set_serial_data(serial, edge_serial);
 
 	status = request_firmware(&fw, fw_name, dev);
-	if (status) {
+	if (status || fw == NULL) {
 		dev_err(&serial->interface->dev,
 				"%s - Failed to load image \"%s\" err %d\n",
 				__func__, fw_name, status);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux