[v2] net/bonding: fix potential out of bounds read

Message ID 1555493621-353-1-git-send-email-radu.nicolau@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] net/bonding: fix potential out of bounds read |

Checks

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

Commit Message

Radu Nicolau April 17, 2019, 9:33 a.m. UTC
  Add validation to pointer constructed from the IPv4 header length
in order to prevent malformed packets from generating a potential
out of bounds memory read.

Fixes: 09150784a776 ("net/bonding: burst mode hash calculation")
Cc: stable@dpdk.org

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
v2: add fixes lines

 drivers/net/bonding/rte_eth_bond_pmd.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
  

Comments

Ferruh Yigit April 17, 2019, 2:25 p.m. UTC | #1
On 4/17/2019 10:33 AM, Radu Nicolau wrote:
> Add validation to pointer constructed from the IPv4 header length
> in order to prevent malformed packets from generating a potential
> out of bounds memory read.
> 
> Fixes: 09150784a776 ("net/bonding: burst mode hash calculation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> v2: add fixes lines
> 
>  drivers/net/bonding/rte_eth_bond_pmd.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
> index b0d191d..25dbddc 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -842,6 +842,7 @@ burst_xmit_l34_hash(struct rte_mbuf **buf, uint16_t nb_pkts,
>  
>  	for (i = 0; i < nb_pkts; i++) {
>  		eth_hdr = rte_pktmbuf_mtod(buf[i], struct ether_hdr *);
> +		size_t pkt_end = (size_t)eth_hdr + rte_pktmbuf_pkt_len(buf[i]);

I guess this should be 'data_len' instead.

>  		proto = eth_hdr->ether_type;
>  		vlan_offset = get_vlan_offset(eth_hdr, &proto);
>  		l3hash = 0;
> @@ -865,13 +866,17 @@ burst_xmit_l34_hash(struct rte_mbuf **buf, uint16_t nb_pkts,
>  					tcp_hdr = (struct tcp_hdr *)
>  						((char *)ipv4_hdr +
>  							ip_hdr_offset);
> -					l4hash = HASH_L4_PORTS(tcp_hdr);
> +					if ((size_t)tcp_hdr + sizeof(*tcp_hdr)
> +							< pkt_end)
> +						l4hash = HASH_L4_PORTS(tcp_hdr);
>  				} else if (ipv4_hdr->next_proto_id ==
>  								IPPROTO_UDP) {
>  					udp_hdr = (struct udp_hdr *)
>  						((char *)ipv4_hdr +
>  							ip_hdr_offset);
> -					l4hash = HASH_L4_PORTS(udp_hdr);
> +					if ((size_t)udp_hdr + sizeof(*udp_hdr)
> +							< pkt_end)
> +						l4hash = HASH_L4_PORTS(udp_hdr);
>  				}
>  			}
>  		} else if  (rte_cpu_to_be_16(ETHER_TYPE_IPv6) == proto) {
> 

Other thing is on action to take when malformed packet is detected, right now it
just prevents the out of bound memory access, but calculated hash will be wrong.
What do you think skipping/dropping the packet in that case, if possible?
  

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b0d191d..25dbddc 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -842,6 +842,7 @@  burst_xmit_l34_hash(struct rte_mbuf **buf, uint16_t nb_pkts,
 
 	for (i = 0; i < nb_pkts; i++) {
 		eth_hdr = rte_pktmbuf_mtod(buf[i], struct ether_hdr *);
+		size_t pkt_end = (size_t)eth_hdr + rte_pktmbuf_pkt_len(buf[i]);
 		proto = eth_hdr->ether_type;
 		vlan_offset = get_vlan_offset(eth_hdr, &proto);
 		l3hash = 0;
@@ -865,13 +866,17 @@  burst_xmit_l34_hash(struct rte_mbuf **buf, uint16_t nb_pkts,
 					tcp_hdr = (struct tcp_hdr *)
 						((char *)ipv4_hdr +
 							ip_hdr_offset);
-					l4hash = HASH_L4_PORTS(tcp_hdr);
+					if ((size_t)tcp_hdr + sizeof(*tcp_hdr)
+							< pkt_end)
+						l4hash = HASH_L4_PORTS(tcp_hdr);
 				} else if (ipv4_hdr->next_proto_id ==
 								IPPROTO_UDP) {
 					udp_hdr = (struct udp_hdr *)
 						((char *)ipv4_hdr +
 							ip_hdr_offset);
-					l4hash = HASH_L4_PORTS(udp_hdr);
+					if ((size_t)udp_hdr + sizeof(*udp_hdr)
+							< pkt_end)
+						l4hash = HASH_L4_PORTS(udp_hdr);
 				}
 			}
 		} else if  (rte_cpu_to_be_16(ETHER_TYPE_IPv6) == proto) {