diff mbox series

[2/2] net/tap: add buffer overflow checks before checksum

Message ID 20181217155005.13457-3-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series prevent out of bounds read with checksum |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson Dec. 17, 2018, 3:50 p.m. UTC 
  The checksum calculation APIs take only the packet headers pointers as
parameters, so they assume that the lengths reported in those headers are
correct. However, a malicious packet could claim to be far larger than it
is, so we need to check the header lengths in the driver before calling
the checksum API.

A better fix would be to allow the lengths to be passed into the API
function, but that would be an API break, so fixing in TAP driver for
now.

CC: stable@dpdk.org
Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/tap/rte_eth_tap.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Ferruh Yigit Dec. 20, 2018, 7:08 p.m. UTC | #1 
On 12/17/2018 3:50 PM, Bruce Richardson wrote:
> The checksum calculation APIs take only the packet headers pointers as
> parameters, so they assume that the lengths reported in those headers are
> correct. However, a malicious packet could claim to be far larger than it
> is, so we need to check the header lengths in the driver before calling
> the checksum API.
> 
> A better fix would be to allow the lengths to be passed into the API
> function, but that would be an API break, so fixing in TAP driver for
> now.
> 
> CC: stable@dpdk.org
> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Wiles, Keith Dec. 20, 2018, 7:33 p.m. UTC | #2 
> On Dec 17, 2018, at 9:50 AM, Richardson, Bruce <bruce.richardson@intel.com> wrote:
> 
> The checksum calculation APIs take only the packet headers pointers as
> parameters, so they assume that the lengths reported in those headers are
> correct. However, a malicious packet could claim to be far larger than it
> is, so we need to check the header lengths in the driver before calling
> the checksum API.
> 
> A better fix would be to allow the lengths to be passed into the API
> function, but that would be an API break, so fixing in TAP driver for
> now.
> 
> CC: stable@dpdk.org
> Fixes: 8ae3023387e9 ("net/tap: add Rx/Tx checksum offload support")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> —

Acked-by: Keith Wiles <keith.wiles@intel.com>

Regards,
Keith
diff mbox series

Patch

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 49afd38dd..0ec030bef 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -281,13 +281,27 @@  tap_verify_csum(struct rte_mbuf *mbuf)
 		l3_len = 4 * (iph->version_ihl & 0xf);
 		if (unlikely(l2_len + l3_len > rte_pktmbuf_data_len(mbuf)))
 			return;
+		/* check that the total length reported by header is not
+		 * greater than the total received size
+		 */
+		if (l2_len + rte_be_to_cpu_16(iph->total_length) >
+				rte_pktmbuf_data_len(mbuf))
+			return;
 
 		cksum = ~rte_raw_cksum(iph, l3_len);
 		mbuf->ol_flags |= cksum ?
 			PKT_RX_IP_CKSUM_BAD :
 			PKT_RX_IP_CKSUM_GOOD;
 	} else if (l3 == RTE_PTYPE_L3_IPV6) {
+		struct ipv6_hdr *iph = l3_hdr;
+
 		l3_len = sizeof(struct ipv6_hdr);
+		/* check that the total length reported by header is not
+		 * greater than the total received size
+		 */
+		if (l2_len + l3_len + rte_be_to_cpu_16(iph->payload_len) >
+				rte_pktmbuf_data_len(mbuf))
+			return;
 	} else {
 		/* IPv6 extensions are not supported */
 		return;