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.
> }
>
> 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--;
>
