[dpdk-dev] [PATCH] ip_frag : Fix double-free of chained mbufs

[dpdk-dev] [PATCH] ip_frag : Fix double-free of chained mbufs

[cychong]

If any fragment hole is found in ipv4_frag_reassemble() and ipv6_frag_reassemble(),
whole ip_frag_pkt mbufs are moved to death-row. Any mbufs already chained to
another mbuf are freed multiple times as there are still in ip_frag_pkt array.

Signed-off-by: cychong <cychong@gmail.com>
---
 lib/librte_ip_frag/ip_frag_common.h      | 4 ++--
 lib/librte_ip_frag/rte_ipv4_reassembly.c | 8 +++++++-
 lib/librte_ip_frag/rte_ipv6_reassembly.c | 8 +++++++-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ip_frag/ip_frag_common.h b/lib/librte_ip_frag/ip_frag_common.h
index cde6ed4..5542a38 100644
--- a/lib/librte_ip_frag/ip_frag_common.h
+++ b/lib/librte_ip_frag/ip_frag_common.h
@@ -76,8 +76,8 @@ struct ip_frag_pkt * ip_frag_lookup(struct rte_ip_frag_tbl *tbl,
 	struct ip_frag_pkt **free, struct ip_frag_pkt **stale);
 
 /* these functions need to be declared here as ip_frag_process relies on them */
-struct rte_mbuf * ipv4_frag_reassemble(const struct ip_frag_pkt *fp);
-struct rte_mbuf * ipv6_frag_reassemble(const struct ip_frag_pkt *fp);
+struct rte_mbuf *ipv4_frag_reassemble(struct ip_frag_pkt *fp);
+struct rte_mbuf *ipv6_frag_reassemble(struct ip_frag_pkt *fp);
 
 
 
diff --git a/lib/librte_ip_frag/rte_ipv4_reassembly.c b/lib/librte_ip_frag/rte_ipv4_reassembly.c
index 26d07f9..e084ca5 100644
--- a/lib/librte_ip_frag/rte_ipv4_reassembly.c
+++ b/lib/librte_ip_frag/rte_ipv4_reassembly.c
@@ -41,11 +41,12 @@
  * Reassemble fragments into one packet.
  */
 struct rte_mbuf *
-ipv4_frag_reassemble(const struct ip_frag_pkt *fp)
+ipv4_frag_reassemble(struct ip_frag_pkt *fp)
 {
 	struct ipv4_hdr *ip_hdr;
 	struct rte_mbuf *m, *prev;
 	uint32_t i, n, ofs, first_len;
+	uint32_t curr_idx = 0;
 
 	first_len = fp->frags[IP_FIRST_FRAG_IDX].len;
 	n = fp->last_idx - 1;
@@ -53,6 +54,7 @@ ipv4_frag_reassemble(const struct ip_frag_pkt *fp)
 	/*start from the last fragment. */
 	m = fp->frags[IP_LAST_FRAG_IDX].mb;
 	ofs = fp->frags[IP_LAST_FRAG_IDX].ofs;
+	curr_idx = IP_LAST_FRAG_IDX;
 
 	while (ofs != first_len) {
 
@@ -67,6 +69,10 @@ ipv4_frag_reassemble(const struct ip_frag_pkt *fp)
 				rte_pktmbuf_adj(m, (uint16_t)(m->l2_len + m->l3_len));
 				rte_pktmbuf_chain(fp->frags[i].mb, m);
 
+				/* this mbuf should not be accessed directly */
+				fp->frags[curr_idx].mb = NULL;
+				curr_idx = i;
+
 				/* update our last fragment and offset. */
 				m = fp->frags[i].mb;
 				ofs = fp->frags[i].ofs;
diff --git a/lib/librte_ip_frag/rte_ipv6_reassembly.c b/lib/librte_ip_frag/rte_ipv6_reassembly.c
index d29cb1d..21a5ef5 100644
--- a/lib/librte_ip_frag/rte_ipv6_reassembly.c
+++ b/lib/librte_ip_frag/rte_ipv6_reassembly.c
@@ -59,13 +59,14 @@ ip_frag_memmove(char *dst, char *src, int len)
  * Reassemble fragments into one packet.
  */
 struct rte_mbuf *
-ipv6_frag_reassemble(const struct ip_frag_pkt *fp)
+ipv6_frag_reassemble(struct ip_frag_pkt *fp)
 {
 	struct ipv6_hdr *ip_hdr;
 	struct ipv6_extension_fragment *frag_hdr;
 	struct rte_mbuf *m, *prev;
 	uint32_t i, n, ofs, first_len;
 	uint32_t last_len, move_len, payload_len;
+	uint32_t curr_idx = 0;
 
 	first_len = fp->frags[IP_FIRST_FRAG_IDX].len;
 	n = fp->last_idx - 1;
@@ -74,6 +75,7 @@ ipv6_frag_reassemble(const struct ip_frag_pkt *fp)
 	m = fp->frags[IP_LAST_FRAG_IDX].mb;
 	ofs = fp->frags[IP_LAST_FRAG_IDX].ofs;
 	last_len = fp->frags[IP_LAST_FRAG_IDX].len;
+	curr_idx = IP_LAST_FRAG_IDX;
 
 	payload_len = ofs + last_len;
 
@@ -90,6 +92,10 @@ ipv6_frag_reassemble(const struct ip_frag_pkt *fp)
 				rte_pktmbuf_adj(m, (uint16_t)(m->l2_len + m->l3_len));
 				rte_pktmbuf_chain(fp->frags[i].mb, m);
 
+				/* this mbuf should not be accessed directly */
+				fp->frags[curr_idx].mb = NULL;
+				curr_idx = i;
+
 				/* update our last fragment and offset. */
 				m = fp->frags[i].mb;
 				ofs = fp->frags[i].ofs;
-- 
1.9.1

Re: [dpdk-dev] [PATCH] ip_frag : Fix double-free of chained mbufs

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of cychong
> Sent: Thursday, April 21, 2016 6:28 PM
> To: dev@dpdk.org
> Cc: cychong@gmail.com
> Subject: [dpdk-dev] [PATCH] ip_frag : Fix double-free of chained mbufs
> 
> If any fragment hole is found in ipv4_frag_reassemble() and ipv6_frag_reassemble(),
> whole ip_frag_pkt mbufs are moved to death-row. Any mbufs already chained to
> another mbuf are freed multiple times as there are still in ip_frag_pkt array.
> 
> Signed-off-by: cychong <cychong@gmail.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [dpdk-dev] [PATCH] ip_frag : Fix double-free of chained mbufs

[Thomas Monjalon]

> > If any fragment hole is found in ipv4_frag_reassemble() and ipv6_frag_reassemble(),
> > whole ip_frag_pkt mbufs are moved to death-row. Any mbufs already chained to
> > another mbuf are freed multiple times as there are still in ip_frag_pkt array.
> > 
> > Signed-off-by: cychong <cychong@gmail.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks