[v2] gro : check for payload length after the trim

Message ID 20221016144305.19249-1-kumaraparmesh92@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] gro : check for payload length after the trim |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/github-robot: build success github build: passed

Commit Message

Kumara Parameshwaran Oct. 16, 2022, 2:43 p.m. UTC
  From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>

When packet is padded with extra bytes the
the validation of the payload length should be done
after the trim operation

Fixes: b8a55871d5af ("gro: trim tail padding bytes")
Cc: stable@dpdk.org

Signed-off-by: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
---
v1:
	If there is padding to the ethernet frame cases where timestamp is disabled
	the packet length should be validated with the total ip length as packet length 
	is used in the GRO merging logic

v2:
	Trim the packet length and then check for the protocol payload validation
 lib/gro/gro_tcp4.c | 11 ++++++-----
 lib/gro/gro_udp4.c | 10 +++++-----
 2 files changed, 11 insertions(+), 10 deletions(-)
  

Comments

Hu, Jiayu Oct. 18, 2022, 8:21 a.m. UTC | #1
> -----Original Message-----
> From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> Sent: Sunday, October 16, 2022 10:43 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>
> Cc: dev@dpdk.org; Kumara Parameshwaran
> <kumaraparamesh92@gmail.com>; stable@dpdk.org
> Subject: [PATCH v2] gro : check for payload length after the trim
> 
> From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> 
> When packet is padded with extra bytes the the validation of the payload
> length should be done after the trim operation
> 
> Fixes: b8a55871d5af ("gro: trim tail padding bytes")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> ---
> v1:
> 	If there is padding to the ethernet frame cases where timestamp is
> disabled
> 	the packet length should be validated with the total ip length as
> packet length
> 	is used in the GRO merging logic
> 
> v2:
> 	Trim the packet length and then check for the protocol payload
> validation  lib/gro/gro_tcp4.c | 11 ++++++-----  lib/gro/gro_udp4.c | 10
> +++++-----
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/gro/gro_tcp4.c b/lib/gro/gro_tcp4.c index
> 8f5e800250..0014096e63 100644
> --- a/lib/gro/gro_tcp4.c
> +++ b/lib/gro/gro_tcp4.c
> @@ -225,6 +225,12 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,
>  	 */
>  	if (tcp_hdr->tcp_flags != RTE_TCP_ACK_FLAG)
>  		return -1;
> +
> +	/* trim the tail padding bytes */
> +	ip_tlen = rte_be_to_cpu_16(ipv4_hdr->total_length);
> +	if (pkt->pkt_len > (uint32_t)(ip_tlen + pkt->l2_len))
> +		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_tlen - pkt->l2_len);
> +
>  	/*
>  	 * Don't process the packet whose payload length is less than or
>  	 * equal to 0.
> @@ -233,11 +239,6 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,
>  	if (tcp_dl <= 0)
>  		return -1;
> 
> -	/* trim the tail padding bytes */
> -	ip_tlen = rte_be_to_cpu_16(ipv4_hdr->total_length);
> -	if (pkt->pkt_len > (uint32_t)(ip_tlen + pkt->l2_len))
> -		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_tlen - pkt->l2_len);
> -
>  	/*
>  	 * Save IPv4 ID for the packet whose DF bit is 0. For the packet
>  	 * whose DF bit is 1, IPv4 ID is ignored.
> diff --git a/lib/gro/gro_udp4.c b/lib/gro/gro_udp4.c index
> 839f9748b7..42596d33b6 100644
> --- a/lib/gro/gro_udp4.c
> +++ b/lib/gro/gro_udp4.c
> @@ -220,6 +220,11 @@ gro_udp4_reassemble(struct rte_mbuf *pkt,
>  	if (!is_ipv4_fragment(ipv4_hdr))
>  		return -1;
> 
> +	ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length);
> +	/* trim the tail padding bytes */
> +	if (pkt->pkt_len > (uint32_t)(ip_dl + pkt->l2_len))
> +		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_dl - pkt->l2_len);
> +
>  	/*
>  	 * Don't process the packet whose payload length is less than or
>  	 * equal to 0.
> @@ -227,14 +232,9 @@ gro_udp4_reassemble(struct rte_mbuf *pkt,
>  	if (pkt->pkt_len <= hdr_len)
>  		return -1;
> 
> -	ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length);
>  	if (ip_dl <= pkt->l3_len)
>  		return -1;
> 
> -	/* trim the tail padding bytes */
> -	if (pkt->pkt_len > (uint32_t)(ip_dl + pkt->l2_len))
> -		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_dl - pkt->l2_len);
> -
>  	ip_dl -= pkt->l3_len;
>  	ip_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);
>  	frag_offset = rte_be_to_cpu_16(ipv4_hdr->fragment_offset);
> --
> 2.25.1

Acked-by: Jiayu Hu <Jiayu.hu@intel.com>

Thanks,
Jiayu
  
Thomas Monjalon Oct. 26, 2022, 3:23 p.m. UTC | #2
> > From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> > 
> > When packet is padded with extra bytes the the validation of the payload
> > length should be done after the trim operation
> > 
> > Fixes: b8a55871d5af ("gro: trim tail padding bytes")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> 
> Acked-by: Jiayu Hu <Jiayu.hu@intel.com>

Applied, thanks.
  

Patch

diff --git a/lib/gro/gro_tcp4.c b/lib/gro/gro_tcp4.c
index 8f5e800250..0014096e63 100644
--- a/lib/gro/gro_tcp4.c
+++ b/lib/gro/gro_tcp4.c
@@ -225,6 +225,12 @@  gro_tcp4_reassemble(struct rte_mbuf *pkt,
 	 */
 	if (tcp_hdr->tcp_flags != RTE_TCP_ACK_FLAG)
 		return -1;
+
+	/* trim the tail padding bytes */
+	ip_tlen = rte_be_to_cpu_16(ipv4_hdr->total_length);
+	if (pkt->pkt_len > (uint32_t)(ip_tlen + pkt->l2_len))
+		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_tlen - pkt->l2_len);
+
 	/*
 	 * Don't process the packet whose payload length is less than or
 	 * equal to 0.
@@ -233,11 +239,6 @@  gro_tcp4_reassemble(struct rte_mbuf *pkt,
 	if (tcp_dl <= 0)
 		return -1;
 
-	/* trim the tail padding bytes */
-	ip_tlen = rte_be_to_cpu_16(ipv4_hdr->total_length);
-	if (pkt->pkt_len > (uint32_t)(ip_tlen + pkt->l2_len))
-		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_tlen - pkt->l2_len);
-
 	/*
 	 * Save IPv4 ID for the packet whose DF bit is 0. For the packet
 	 * whose DF bit is 1, IPv4 ID is ignored.
diff --git a/lib/gro/gro_udp4.c b/lib/gro/gro_udp4.c
index 839f9748b7..42596d33b6 100644
--- a/lib/gro/gro_udp4.c
+++ b/lib/gro/gro_udp4.c
@@ -220,6 +220,11 @@  gro_udp4_reassemble(struct rte_mbuf *pkt,
 	if (!is_ipv4_fragment(ipv4_hdr))
 		return -1;
 
+	ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length);
+	/* trim the tail padding bytes */
+	if (pkt->pkt_len > (uint32_t)(ip_dl + pkt->l2_len))
+		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_dl - pkt->l2_len);
+
 	/*
 	 * Don't process the packet whose payload length is less than or
 	 * equal to 0.
@@ -227,14 +232,9 @@  gro_udp4_reassemble(struct rte_mbuf *pkt,
 	if (pkt->pkt_len <= hdr_len)
 		return -1;
 
-	ip_dl = rte_be_to_cpu_16(ipv4_hdr->total_length);
 	if (ip_dl <= pkt->l3_len)
 		return -1;
 
-	/* trim the tail padding bytes */
-	if (pkt->pkt_len > (uint32_t)(ip_dl + pkt->l2_len))
-		rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_dl - pkt->l2_len);
-
 	ip_dl -= pkt->l3_len;
 	ip_id = rte_be_to_cpu_16(ipv4_hdr->packet_id);
 	frag_offset = rte_be_to_cpu_16(ipv4_hdr->fragment_offset);