diff mbox

af9035: Add USB read checksumming

Message ID 20120401223348.5f163b5d@milhouse (mailing list archive)
State Accepted, archived
Headers

Commit Message

Michael Büsch April 1, 2012, 8:33 p.m. UTC 
  This adds USB message read checksumming to protect against
device and bus errors.
It also adds a read length check to avoid returning garbage from
the buffer, if the device truncated the message.

Signed-off-by: Michael Buesch <m@bues.ch>

---

Comments

Antti Palosaari April 1, 2012, 9:41 p.m. UTC | #1 
On 01.04.2012 23:33, Michael Büsch wrote:
> This adds USB message read checksumming to protect against
> device and bus errors.
> It also adds a read length check to avoid returning garbage from
> the buffer, if the device truncated the message.
>
> Signed-off-by: Michael Buesch<m@bues.ch>

Applied thanks!

http://git.linuxtv.org/anttip/media_tree.git/shortlog/refs/heads/af9035_experimental

Personally didn't see that very important but I will not see any reason 
not to implement it still :)

regards
Antti
diff mbox

Patch

Index: linux/drivers/media/dvb/dvb-usb/af9035.c
===================================================================
--- linux.orig/drivers/media/dvb/dvb-usb/af9035.c	2012-04-01 21:44:27.767000731 +0200
+++ linux/drivers/media/dvb/dvb-usb/af9035.c	2012-04-01 22:26:46.020185359 +0200
@@ -37,6 +37,22 @@ 
 	}
 };
 
+static u16 af9035_checksum(const u8 *buf, size_t len)
+{
+	size_t i;
+	u16 checksum = 0;
+
+	for (i = 1; i < len; i++) {
+		if (i % 2)
+			checksum += buf[i] << 8;
+		else
+			checksum += buf[i];
+	}
+	checksum = ~checksum;
+
+	return checksum;
+}
+
 static int af9035_ctrl_msg(struct usb_device *udev, struct usb_req *req)
 {
 #define BUF_LEN 63
@@ -45,11 +61,11 @@ 
 #define CHECKSUM_LEN 2
 #define USB_TIMEOUT 2000
 
-	int ret, i, act_len;
+	int ret, act_len;
 	u8 buf[BUF_LEN];
 	u32 msg_len;
 	static u8 seq; /* packet sequence number */
-	u16 checksum = 0;
+	u16 checksum, tmpsum;
 
 	/* buffer overflow check */
 	if (req->wlen > (BUF_LEN - REQ_HDR_LEN - CHECKSUM_LEN) ||
@@ -70,14 +86,7 @@ 
 		memcpy(&buf[4], req->wbuf, req->wlen);
 
 	/* calc and add checksum */
-	for (i = 1; i < buf[0]-1; i++) {
-		if (i % 2)
-			checksum += buf[i] << 8;
-		else
-			checksum += buf[i];
-	}
-	checksum = ~checksum;
-
+	checksum = af9035_checksum(buf, buf[0] - 1);
 	buf[buf[0]-1] = (checksum >> 8);
 	buf[buf[0]-0] = (checksum & 0xff);
 
@@ -107,7 +116,23 @@ 
 		ret = -EIO;
 		goto err_mutex_unlock;
 	}
+	if (act_len != msg_len) {
+		err("recv bulk message truncated (%d != %u)\n",
+		    act_len, (unsigned int)msg_len);
+		ret = -EIO;
+		goto err_mutex_unlock;
+	}
 
+	/* verify checksum */
+	checksum = af9035_checksum(buf, act_len - 2);
+	tmpsum = (buf[act_len - 2] << 8) | buf[act_len - 1];
+	if (tmpsum != checksum) {
+		err("%s: command=%02X checksum mismatch (%04X != %04X)\n",
+		    __func__, req->cmd,
+		    (unsigned int)tmpsum, (unsigned int)checksum);
+		ret = -EIO;
+		goto err_mutex_unlock;
+	}
 	/* check status */
 	if (buf[2]) {
 		pr_debug("%s: command=%02x failed fw error=%d\n", __func__,