[v2] net/bonding: fix potential out of bounds read
Checks
Commit Message
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
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?
@@ -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) {