cmhi
/
f-stack
mirror of https://github.com/F-Stack/f-stack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 <yonghaoz1994@gmail.com>
Signed-off-by: Jianfeng Tan <henry.tjf@antgroup.com>
This commit is contained in:
Jianfeng Tan 2020-11-27 14:45:00 +08:00 committed by fengbojiang
parent fc7cff57bb
commit e6161e2b0f
3 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
lib/ff_dpdk_if.c
View File

@ -1150,7 +1150,7 @@ ff_veth_input(const struct ff_dpdk_if_context *ctx, struct rte_mbuf *pkt)
data = rte_pktmbuf_mtod(pn, void*); data = rte_pktmbuf_mtod(pn, void*);
len = rte_pktmbuf_data_len(pn); 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) { if (mb == NULL) {
ff_mbuf_free(hdr); ff_mbuf_free(hdr);
rte_pktmbuf_free(pkt); rte_pktmbuf_free(pkt);
@ -1967,7 +1967,7 @@ ff_dpdk_run(loop_func_t loop, void *arg) {
void void
ff_dpdk_pktmbuf_free(void *m) ff_dpdk_pktmbuf_free(void *m)
{ {
rte_pktmbuf_free((struct rte_mbuf *)m); rte_pktmbuf_free_seg((struct rte_mbuf *)m);
} }
static uint32_t static uint32_t

6
lib/ff_veth.c
View File

@ -209,16 +209,16 @@ ff_mbuf_gethdr(void *pkt, uint16_t total, void *data,
} }
void * 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); struct mbuf *mb = m_get(M_NOWAIT, MT_DATA);
if (mb == NULL) { if (mb == NULL) {
return 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_next = NULL;
mb->m_nextpkt = NULL; mb->m_nextpkt = NULL;

2
lib/ff_veth.h
View File

@ -33,7 +33,7 @@ int ff_veth_detach(void *arg);
void *ff_mbuf_gethdr(void *pkt, uint16_t total, void *data, void *ff_mbuf_gethdr(void *pkt, uint16_t total, void *data,
uint16_t len, uint8_t rx_csum); 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); void ff_mbuf_free(void *m);
int ff_mbuf_copydata(void *m, void *data, int off, int len); int ff_mbuf_copydata(void *m, void *data, int off, int len);