* [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read
2019-04-15 9:27 [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read Radu Nicolau
@ 2019-04-15 9:27 ` Radu Nicolau
2019-04-16 16:07 ` Ferruh Yigit
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Radu Nicolau @ 2019-04-15 9:27 UTC (permalink / raw)
To: dev; +Cc: declan.doherty, chas3, Radu Nicolau
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.
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
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]);
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) {
--
2.7.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read
2019-04-15 9:27 [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read Radu Nicolau
2019-04-15 9:27 ` Radu Nicolau
@ 2019-04-16 16:07 ` Ferruh Yigit
2019-04-16 16:07 ` Ferruh Yigit
2019-04-17 9:33 ` [dpdk-dev] [PATCH v2] " Radu Nicolau
2019-04-17 14:36 ` [dpdk-dev] [PATCH v3] " Radu Nicolau
3 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-16 16:07 UTC (permalink / raw)
To: Radu Nicolau, dev; +Cc: declan.doherty, chas3
On 4/15/2019 10:27 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.
Can you please add fixes lines?
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> 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]);
> 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) {
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read
2019-04-16 16:07 ` Ferruh Yigit
@ 2019-04-16 16:07 ` Ferruh Yigit
0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-16 16:07 UTC (permalink / raw)
To: Radu Nicolau, dev; +Cc: declan.doherty, chas3
On 4/15/2019 10:27 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.
Can you please add fixes lines?
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> 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]);
> 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) {
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v2] net/bonding: fix potential out of bounds read
2019-04-15 9:27 [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read Radu Nicolau
2019-04-15 9:27 ` Radu Nicolau
2019-04-16 16:07 ` Ferruh Yigit
@ 2019-04-17 9:33 ` Radu Nicolau
2019-04-17 9:33 ` Radu Nicolau
2019-04-17 14:25 ` Ferruh Yigit
2019-04-17 14:36 ` [dpdk-dev] [PATCH v3] " Radu Nicolau
3 siblings, 2 replies; 16+ messages in thread
From: Radu Nicolau @ 2019-04-17 9:33 UTC (permalink / raw)
To: dev; +Cc: declan.doherty, chas3, ferruh.yigit, Radu Nicolau, stable
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]);
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) {
--
2.7.5
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH v2] net/bonding: fix potential out of bounds read
2019-04-17 9:33 ` [dpdk-dev] [PATCH v2] " Radu Nicolau
@ 2019-04-17 9:33 ` Radu Nicolau
2019-04-17 14:25 ` Ferruh Yigit
1 sibling, 0 replies; 16+ messages in thread
From: Radu Nicolau @ 2019-04-17 9:33 UTC (permalink / raw)
To: dev; +Cc: declan.doherty, chas3, ferruh.yigit, Radu Nicolau, stable
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]);
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) {
--
2.7.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v2] net/bonding: fix potential out of bounds read
2019-04-17 9:33 ` [dpdk-dev] [PATCH v2] " Radu Nicolau
2019-04-17 9:33 ` Radu Nicolau
@ 2019-04-17 14:25 ` Ferruh Yigit
2019-04-17 14:25 ` Ferruh Yigit
1 sibling, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-17 14:25 UTC (permalink / raw)
To: Radu Nicolau, dev; +Cc: declan.doherty, chas3, stable
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?
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v2] net/bonding: fix potential out of bounds read
2019-04-17 14:25 ` Ferruh Yigit
@ 2019-04-17 14:25 ` Ferruh Yigit
0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-17 14:25 UTC (permalink / raw)
To: Radu Nicolau, dev; +Cc: declan.doherty, chas3, stable
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?
^ permalink raw reply [flat|nested] 16+ messages in thread
* [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-15 9:27 [dpdk-dev] [PATCH] net/bonding: fix potential out of bounds read Radu Nicolau
` (2 preceding siblings ...)
2019-04-17 9:33 ` [dpdk-dev] [PATCH v2] " Radu Nicolau
@ 2019-04-17 14:36 ` Radu Nicolau
2019-04-17 14:36 ` Radu Nicolau
2019-04-18 18:41 ` Ferruh Yigit
3 siblings, 2 replies; 16+ messages in thread
From: Radu Nicolau @ 2019-04-17 14:36 UTC (permalink / raw)
To: dev; +Cc: declan.doherty, chas3, ferruh.yigit, Radu Nicolau, stable
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
v3: fix buffer end calculation
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..2b7f2b3 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_data_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) {
--
2.7.5
^ permalink raw reply [flat|nested] 16+ messages in thread* [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-17 14:36 ` [dpdk-dev] [PATCH v3] " Radu Nicolau
@ 2019-04-17 14:36 ` Radu Nicolau
2019-04-18 18:41 ` Ferruh Yigit
1 sibling, 0 replies; 16+ messages in thread
From: Radu Nicolau @ 2019-04-17 14:36 UTC (permalink / raw)
To: dev; +Cc: declan.doherty, chas3, ferruh.yigit, Radu Nicolau, stable
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
v3: fix buffer end calculation
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..2b7f2b3 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_data_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) {
--
2.7.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-17 14:36 ` [dpdk-dev] [PATCH v3] " Radu Nicolau
2019-04-17 14:36 ` Radu Nicolau
@ 2019-04-18 18:41 ` Ferruh Yigit
2019-04-18 18:41 ` Ferruh Yigit
2019-04-18 22:57 ` Chas Williams
1 sibling, 2 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 18:41 UTC (permalink / raw)
To: chas3; +Cc: Radu Nicolau, dev, declan.doherty, stable
On 4/17/2019 3:36 PM, 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>
Hi Chas,
Do you have any objection on the patch?
Functionally looks correct to me, but additional checks in datapath perhaps can
be a concern, if not I am for getting the patch.
> ---
> v2: add fixes lines
> v3: fix buffer end calculation
>
> 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..2b7f2b3 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_data_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) {
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-18 18:41 ` Ferruh Yigit
@ 2019-04-18 18:41 ` Ferruh Yigit
2019-04-18 22:57 ` Chas Williams
1 sibling, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-18 18:41 UTC (permalink / raw)
To: chas3; +Cc: Radu Nicolau, dev, declan.doherty, stable
On 4/17/2019 3:36 PM, 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>
Hi Chas,
Do you have any objection on the patch?
Functionally looks correct to me, but additional checks in datapath perhaps can
be a concern, if not I am for getting the patch.
> ---
> v2: add fixes lines
> v3: fix buffer end calculation
>
> 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..2b7f2b3 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_data_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) {
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-18 18:41 ` Ferruh Yigit
2019-04-18 18:41 ` Ferruh Yigit
@ 2019-04-18 22:57 ` Chas Williams
2019-04-18 22:57 ` Chas Williams
2019-04-19 9:43 ` Ferruh Yigit
1 sibling, 2 replies; 16+ messages in thread
From: Chas Williams @ 2019-04-18 22:57 UTC (permalink / raw)
To: dev
On 4/18/19 2:41 PM, Ferruh Yigit wrote:
> On 4/17/2019 3:36 PM, 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>
>
> Hi Chas,
>
> Do you have any objection on the patch?
> Functionally looks correct to me, but additional checks in datapath perhaps can
> be a concern, if not I am for getting the patch.
I don't think the calculation is a huge concern.
Acked-by: Chas Williams <chas3@att.com>
>
>> ---
>> v2: add fixes lines
>> v3: fix buffer end calculation
>>
>> 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..2b7f2b3 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_data_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) {
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-18 22:57 ` Chas Williams
@ 2019-04-18 22:57 ` Chas Williams
2019-04-19 9:43 ` Ferruh Yigit
1 sibling, 0 replies; 16+ messages in thread
From: Chas Williams @ 2019-04-18 22:57 UTC (permalink / raw)
To: dev
On 4/18/19 2:41 PM, Ferruh Yigit wrote:
> On 4/17/2019 3:36 PM, 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>
>
> Hi Chas,
>
> Do you have any objection on the patch?
> Functionally looks correct to me, but additional checks in datapath perhaps can
> be a concern, if not I am for getting the patch.
I don't think the calculation is a huge concern.
Acked-by: Chas Williams <chas3@att.com>
>
>> ---
>> v2: add fixes lines
>> v3: fix buffer end calculation
>>
>> 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..2b7f2b3 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_data_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) {
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-18 22:57 ` Chas Williams
2019-04-18 22:57 ` Chas Williams
@ 2019-04-19 9:43 ` Ferruh Yigit
2019-04-19 9:43 ` Ferruh Yigit
1 sibling, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-19 9:43 UTC (permalink / raw)
To: Chas Williams, dev
On 4/18/2019 11:57 PM, Chas Williams wrote:
>
>
> On 4/18/19 2:41 PM, Ferruh Yigit wrote:
>> On 4/17/2019 3:36 PM, 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>
>>
>> Hi Chas,
>>
>> Do you have any objection on the patch?
>> Functionally looks correct to me, but additional checks in datapath perhaps can
>> be a concern, if not I am for getting the patch.
>
> I don't think the calculation is a huge concern.
>
> Acked-by: Chas Williams <chas3@att.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [dpdk-dev] [PATCH v3] net/bonding: fix potential out of bounds read
2019-04-19 9:43 ` Ferruh Yigit
@ 2019-04-19 9:43 ` Ferruh Yigit
0 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2019-04-19 9:43 UTC (permalink / raw)
To: Chas Williams, dev
On 4/18/2019 11:57 PM, Chas Williams wrote:
>
>
> On 4/18/19 2:41 PM, Ferruh Yigit wrote:
>> On 4/17/2019 3:36 PM, 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>
>>
>> Hi Chas,
>>
>> Do you have any objection on the patch?
>> Functionally looks correct to me, but additional checks in datapath perhaps can
>> be a concern, if not I am for getting the patch.
>
> I don't think the calculation is a huge concern.
>
> Acked-by: Chas Williams <chas3@att.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread