On Mon, 6 May 2019 at 10:26, Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > > On 04/30/2019 02:45 PM, Björn Töpel wrote: > > From: Björn Töpel <bjorn.topel@xxxxxxxxx> > > > > When unmapping the AF_XDP memory regions used for the rings, an > > invalid address was passed to the munmap() calls. Instead of passing > > the beginning of the memory region, the descriptor region was passed > > to munmap. > > > > When the userspace application tried to tear down an AF_XDP socket, > > the operation failed and the application would still have a reference > > to socket it wished to get rid of. > > > > Reported-by: William Tu <u9012063@xxxxxxxxx> > > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets") > > Signed-off-by: Björn Töpel <bjorn.topel@xxxxxxxxx> > [...] > > out_mmap_tx: > > if (tx) > > - munmap(xsk->tx, > > - off.tx.desc + > > + munmap(tx_map, off.tx.desc + > > xsk->config.tx_size * sizeof(struct xdp_desc)); > > out_mmap_rx: > > if (rx) > > - munmap(xsk->rx, > > - off.rx.desc + > > + munmap(rx_map, off.rx.desc + > > xsk->config.rx_size * sizeof(struct xdp_desc)); > > out_socket: > > if (--umem->refcount) > > @@ -684,10 +681,12 @@ int xsk_umem__delete(struct xsk_umem *umem) > > optlen = sizeof(off); > > err = getsockopt(umem->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen); > > if (!err) { > > - munmap(umem->fill->ring, > > - off.fr.desc + umem->config.fill_size * sizeof(__u64)); > > - munmap(umem->comp->ring, > > - off.cr.desc + umem->config.comp_size * sizeof(__u64)); > > + (void)munmap(umem->fill->ring - off.fr.desc, > > + off.fr.desc + > > + umem->config.fill_size * sizeof(__u64)); > > + (void)munmap(umem->comp->ring - off.cr.desc, > > + off.cr.desc + > > + umem->config.comp_size * sizeof(__u64)); > > What's the rationale to cast to void here and other places (e.g. below)? > If there's no proper reason, then lets remove it. Given the patch has already > been applied, please send a follow up. Thanks. > The rationale was to make it explicit that the return value is *not* cared about. If this is not common practice, I'll remove it in a follow up! Thank, Björn > > } > > > > close(umem->fd); > > @@ -698,6 +697,7 @@ int xsk_umem__delete(struct xsk_umem *umem) > > > > void xsk_socket__delete(struct xsk_socket *xsk) > > { > > + size_t desc_sz = sizeof(struct xdp_desc); > > struct xdp_mmap_offsets off; > > socklen_t optlen; > > int err; > > @@ -710,14 +710,17 @@ void xsk_socket__delete(struct xsk_socket *xsk) > > optlen = sizeof(off); > > err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen); > > if (!err) { > > - if (xsk->rx) > > - munmap(xsk->rx->ring, > > - off.rx.desc + > > - xsk->config.rx_size * sizeof(struct xdp_desc)); > > - if (xsk->tx) > > - munmap(xsk->tx->ring, > > - off.tx.desc + > > - xsk->config.tx_size * sizeof(struct xdp_desc)); > > + if (xsk->rx) { > > + (void)munmap(xsk->rx->ring - off.rx.desc, > > + off.rx.desc + > > + xsk->config.rx_size * desc_sz); > > + } > > + if (xsk->tx) { > > + (void)munmap(xsk->tx->ring - off.tx.desc, > > + off.tx.desc + > > + xsk->config.tx_size * desc_sz); > > + } > > + > > } > > > > xsk->umem->refcount--; > > >