Skip to content

Commit 664b9bf

Browse files
committed
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 <[email protected]> Signed-off-by: Jianfeng Tan <[email protected]>
1 parent 7e2f640 commit 664b9bf

File tree

3 files changed

+6
-6
lines changed

3 files changed

+6
-6
lines changed

lib/ff_dpdk_if.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,7 @@ ff_veth_input(const struct ff_dpdk_if_context *ctx, struct rte_mbuf *pkt)
11501150
data = rte_pktmbuf_mtod(pn, void*);
11511151
len = rte_pktmbuf_data_len(pn);
11521152

1153-
void *mb = ff_mbuf_get(prev, data, len);
1153+
void *mb = ff_mbuf_get(prev, pn, data, len);
11541154
if (mb == NULL) {
11551155
ff_mbuf_free(hdr);
11561156
rte_pktmbuf_free(pkt);
@@ -1967,7 +1967,7 @@ ff_dpdk_run(loop_func_t loop, void *arg) {
19671967
void
19681968
ff_dpdk_pktmbuf_free(void *m)
19691969
{
1970-
rte_pktmbuf_free((struct rte_mbuf *)m);
1970+
rte_pktmbuf_free_seg((struct rte_mbuf *)m);
19711971
}
19721972

19731973
static uint32_t

lib/ff_veth.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,16 @@ ff_mbuf_gethdr(void *pkt, uint16_t total, void *data,
209209
}
210210

211211
void *
212-
ff_mbuf_get(void *m, void *data, uint16_t len)
212+
ff_mbuf_get(void *p, void *m, void *data, uint16_t len)
213213
{
214-
struct mbuf *prev = (struct mbuf *)m;
214+
struct mbuf *prev = (struct mbuf *)p;
215215
struct mbuf *mb = m_get(M_NOWAIT, MT_DATA);
216216

217217
if (mb == NULL) {
218218
return NULL;
219219
}
220220

221-
m_extadd(mb, data, len, NULL, NULL, NULL, 0, 0);
221+
m_extadd(mb, data, len, ff_mbuf_ext_free, m, NULL, 0, EXT_DISPOSABLE);
222222

223223
mb->m_next = NULL;
224224
mb->m_nextpkt = NULL;

lib/ff_veth.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ int ff_veth_detach(void *arg);
3333

3434
void *ff_mbuf_gethdr(void *pkt, uint16_t total, void *data,
3535
uint16_t len, uint8_t rx_csum);
36-
void *ff_mbuf_get(void *m, void *data, uint16_t len);
36+
void *ff_mbuf_get(void *p, void *m, void *data, uint16_t len);
3737
void ff_mbuf_free(void *m);
3838

3939
int ff_mbuf_copydata(void *m, void *data, int off, int len);

0 commit comments

Comments
 (0)