From 664b9bff710c8845156672125e8ae8738bc340d6 Mon Sep 17 00:00:00 2001 From: Jianfeng Tan Date: Fri, 27 Nov 2020 14:45:00 +0800 Subject: [PATCH] fix use after free issue in mbuf free Two kinds of mbuf are used in f-stack: freebsd mbuf and dpdk mbuf. freebsd mbufs are metadata used in freebsd stack, and their data pointers (m_data) point to dpdk mbuf's data (buf_addr). And they have their own chain, like this: bsd_mbuf1 -> bsd_mbuf2 -> bsd_mbuf3 \ \ \ dpdk_mbuf1 -> dpdk_mbuf2 -> dpdk_mbuf3 Considering the map relationship, - m_freem() is corresponding to rte_pktmbuf_free(), is to free the whole chain of mbufs. - m_free() is corresponding to rte_pktmbuf_free_seg(), is to free the specified mbuf segment. The current implementation in f-stack uses rte_pktmbuf_free() for m_free(). This leads to mbufs, which are still in use, be freed unexpectedly. For example, if the bsd_mbuf1 is trimed into zero length, bsd will invoke m_free() to free the specified segment, however, the whole mbuf chain is freed by calling rte_pktmbuf_free(). #0 rte_pktmbuf_free (m=0x22006fb480) #1 in ff_dpdk_pktmbuf_free (m=0x22006fb480) #2 in ff_mbuf_ext_free (m=0x7ffff7f82800, arg1=0x22006fb480, arg2=0x0) #3 in mb_free_ext (m=0x7ffff7f82800) #4 in m_free (m=0x7ffff7f82800) #5 in sbcompress (sb=, m=0x7ffff7f82800, n=) #6 in sbappendstream_locked (sb=, m=0x7ffff7f82800, flags=0) The fix is straightforward. Use the correct API for segment free. Reported-by: Yong-Hao Zou Signed-off-by: Jianfeng Tan --- lib/ff_dpdk_if.c | 4 ++-- lib/ff_veth.c | 6 +++--- lib/ff_veth.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ff_dpdk_if.c b/lib/ff_dpdk_if.c index 882a6f846..47282c0e7 100644 --- a/lib/ff_dpdk_if.c +++ b/lib/ff_dpdk_if.c @@ -1150,7 +1150,7 @@ ff_veth_input(const struct ff_dpdk_if_context *ctx, struct rte_mbuf *pkt) data = rte_pktmbuf_mtod(pn, void*); len = rte_pktmbuf_data_len(pn); - void *mb = ff_mbuf_get(prev, data, len); + void *mb = ff_mbuf_get(prev, pn, data, len); if (mb == NULL) { ff_mbuf_free(hdr); rte_pktmbuf_free(pkt); @@ -1967,7 +1967,7 @@ ff_dpdk_run(loop_func_t loop, void *arg) { void ff_dpdk_pktmbuf_free(void *m) { - rte_pktmbuf_free((struct rte_mbuf *)m); + rte_pktmbuf_free_seg((struct rte_mbuf *)m); } static uint32_t diff --git a/lib/ff_veth.c b/lib/ff_veth.c index 11c9c1929..778dd4997 100644 --- a/lib/ff_veth.c +++ b/lib/ff_veth.c @@ -209,16 +209,16 @@ ff_mbuf_gethdr(void *pkt, uint16_t total, void *data, } void * -ff_mbuf_get(void *m, void *data, uint16_t len) +ff_mbuf_get(void *p, void *m, void *data, uint16_t len) { - struct mbuf *prev = (struct mbuf *)m; + struct mbuf *prev = (struct mbuf *)p; struct mbuf *mb = m_get(M_NOWAIT, MT_DATA); if (mb == NULL) { return NULL; } - m_extadd(mb, data, len, NULL, NULL, NULL, 0, 0); + m_extadd(mb, data, len, ff_mbuf_ext_free, m, NULL, 0, EXT_DISPOSABLE); mb->m_next = NULL; mb->m_nextpkt = NULL; diff --git a/lib/ff_veth.h b/lib/ff_veth.h index 14721ed15..713ec6d16 100644 --- a/lib/ff_veth.h +++ b/lib/ff_veth.h @@ -33,7 +33,7 @@ int ff_veth_detach(void *arg); void *ff_mbuf_gethdr(void *pkt, uint16_t total, void *data, uint16_t len, uint8_t rx_csum); -void *ff_mbuf_get(void *m, void *data, uint16_t len); +void *ff_mbuf_get(void *p, void *m, void *data, uint16_t len); void ff_mbuf_free(void *m); int ff_mbuf_copydata(void *m, void *data, int off, int len);