Vulnerability: The saa7164_bus_get function in drivers/media/pci/saa7164/saa7164-bus.c in the Linux kernel through 4.11.5 allows local users to cause a denial of service (out-of-bounds array access) or possibly have unspecified other impact by changing a certain sequence-number value, aka a "double fetch" vulnerability.
Vulnerability ID: CVE-2017-8831
CWE: CWE-125
Severity score: 10
Project: https://github.com/stoth68000/media-tree
Published: 2017-05-08
File: drivers/media/pci/saa7164/saa7164-bus.c
Commit message: [PATCH] saa7164: Bug - Double fetch PCIe access condition

Avoid a double fetch by reusing the values from the prior transfer.

Originally reported via https://bugzilla.kernel.org/show_bug.cgi?id=195559

Thanks to Pengfei Wang <wpengfeinudt@gmail.com> for reporting.

Signed-off-by: Steven Toth <stoth@kernellabs.com>
Commit author: Steven Toth

Vulnerable version (before fix):
 int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg, 	msg_tmp.size = le16_to_cpu((__force __le16)msg_tmp.size); 	msg_tmp.command = le32_to_cpu((__force __le32)msg_tmp.command); 	msg_tmp.controlselector = le16_to_cpu((__force __le16)msg_tmp.controlselector);  	/* No need to update the read positions, because this was a peek */ 	/* If the caller specifically want to peek, return */ 	if (peekonly) { 		memcpy(msg, &msg_tmp, sizeof(*msg)); 		goto peekout; 	}  int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg, 		space_rem = bus->m_dwSizeGetRing - curr_grp;  		if (space_rem < sizeof(*msg)) { 			/* msg wraps around the ring */ 			memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, space_rem); 			memcpy_fromio((u8 *)msg + space_rem, bus->m_pdwGetRing, 				sizeof(*msg) - space_rem); 			if (buf) 				memcpy_fromio(buf, bus->m_pdwGetRing + sizeof(*msg) - 					space_rem, buf_size);  		} else if (space_rem == sizeof(*msg)) { 			memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, sizeof(*msg)); 			if (buf) 				memcpy_fromio(buf, bus->m_pdwGetRing, buf_size); 		} else { 			/* Additional data wraps around the ring */ 			memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, sizeof(*msg)); 			if (buf) { 				memcpy_fromio(buf, bus->m_pdwGetRing + curr_grp + 					sizeof(*msg), space_rem - sizeof(*msg)); int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg,  	} else { 		/* No wrapping */ 		memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, sizeof(*msg)); 		if (buf) 			memcpy_fromio(buf, bus->m_pdwGetRing + curr_grp + sizeof(*msg), 				buf_size); 	} 	/* Convert from little endian to CPU */ 	msg->size = le16_to_cpu((__force __le16)msg->size); 	msg->command = le32_to_cpu((__force __le32)msg->command); 	msg->controlselector = le16_to_cpu((__force __le16)msg->controlselector);  	/* Update the read positions, adjusting the ring */ 	saa7164_writel(bus->m_dwGetReadPos, new_grp);

Fixed version (after fix):
 int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg, 	msg_tmp.size = le16_to_cpu((__force __le16)msg_tmp.size); 	msg_tmp.command = le32_to_cpu((__force __le32)msg_tmp.command); 	msg_tmp.controlselector = le16_to_cpu((__force __le16)msg_tmp.controlselector); 	memcpy(msg, &msg_tmp, sizeof(*msg));  	/* No need to update the read positions, because this was a peek */ 	/* If the caller specifically want to peek, return */ 	if (peekonly) { 		goto peekout; 	}  int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg, 		space_rem = bus->m_dwSizeGetRing - curr_grp;  		if (space_rem < sizeof(*msg)) { 			if (buf) 				memcpy_fromio(buf, bus->m_pdwGetRing + sizeof(*msg) - 					space_rem, buf_size);  		} else if (space_rem == sizeof(*msg)) { 			if (buf) 				memcpy_fromio(buf, bus->m_pdwGetRing, buf_size); 		} else { 			/* Additional data wraps around the ring */ 			if (buf) { 				memcpy_fromio(buf, bus->m_pdwGetRing + curr_grp + 					sizeof(*msg), space_rem - sizeof(*msg)); int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg,  	} else { 		/* No wrapping */ 		if (buf) 			memcpy_fromio(buf, bus->m_pdwGetRing + curr_grp + sizeof(*msg), 				buf_size); 	}  	/* Update the read positions, adjusting the ring */ 	saa7164_writel(bus->m_dwGetReadPos, new_grp);

Patch:
patch: @@ -389,11 +389,11 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg,
 	msg_tmp.size = le16_to_cpu((__force __le16)msg_tmp.size);
 	msg_tmp.command = le32_to_cpu((__force __le32)msg_tmp.command);
 	msg_tmp.controlselector = le16_to_cpu((__force __le16)msg_tmp.controlselector);
+	memcpy(msg, &msg_tmp, sizeof(*msg));
 
 	/* No need to update the read positions, because this was a peek */
 	/* If the caller specifically want to peek, return */
 	if (peekonly) {
-		memcpy(msg, &msg_tmp, sizeof(*msg));
 		goto peekout;
 	}
 
@@ -438,21 +438,15 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg,
 		space_rem = bus->m_dwSizeGetRing - curr_grp;
 
 		if (space_rem < sizeof(*msg)) {
-			/* msg wraps around the ring */
-			memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, space_rem);
-			memcpy_fromio((u8 *)msg + space_rem, bus->m_pdwGetRing,
-				sizeof(*msg) - space_rem);
 			if (buf)
 				memcpy_fromio(buf, bus->m_pdwGetRing + sizeof(*msg) -
 					space_rem, buf_size);
 
 		} else if (space_rem == sizeof(*msg)) {
-			memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, sizeof(*msg));
 			if (buf)
 				memcpy_fromio(buf, bus->m_pdwGetRing, buf_size);
 		} else {
 			/* Additional data wraps around the ring */
-			memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, sizeof(*msg));
 			if (buf) {
 				memcpy_fromio(buf, bus->m_pdwGetRing + curr_grp +
 					sizeof(*msg), space_rem - sizeof(*msg));
@@ -465,15 +459,10 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct tmComResInfo* msg,
 
 	} else {
 		/* No wrapping */
-		memcpy_fromio(msg, bus->m_pdwGetRing + curr_grp, sizeof(*msg));
 		if (buf)
 			memcpy_fromio(buf, bus->m_pdwGetRing + curr_grp + sizeof(*msg),
 				buf_size);
 	}
-	/* Convert from little endian to CPU */
-	msg->size = le16_to_cpu((__force __le16)msg->size);
-	msg->command = le32_to_cpu((__force __le32)msg->command);
-	msg->controlselector = le16_to_cpu((__force __le16)msg->controlselector);
 
 	/* Update the read positions, adjusting the ring */
 	saa7164_writel(bus->m_dwGetReadPos, new_grp);
path: drivers/media/pci/saa7164/saa7164-bus.c