Commit 6dcc4e38 authored by Alexei Starovoitov's avatar Alexei Starovoitov

Merge branch 'AF_XDP Socket Creation Fixes'

Ciara Loftus says:

====================

This series fixes some issues around socket creation for AF_XDP.

Patch 1 fixes a potential NULL pointer dereference in
xsk_socket__create_shared.

Patch 2 ensures that the umem passed to xsk_socket__create(_shared)
remains unchanged in event of failure.

Patch 3 makes it possible for xsk_socket__create(_shared) to
succeed even if the rx and tx XDP rings have already been set up by
introducing a new fields to struct xsk_umem which represent the ring
setup status for the xsk which shares the fd with the umem.

v3->v4:
* Reduced nesting in xsk_put_ctx as suggested by Alexei.
* Use bools instead of a u8 and flags to represent the
  ring setup status as suggested by Björn.

v2->v3:
* Instead of ignoring the return values of the setsockopt calls, introduce
  a new flag to determine whether or not to call them based on the ring
  setup status as suggested by Alexei.

v1->v2:
* Simplified restoring the _save pointers as suggested by Magnus.
* Fixed the condition which determines whether to unmap umem rings
 when socket create fails.

====================
Acked-by: default avatarBjörn Töpel <bjorn@kernel.org>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents d37300ed ca7a83e2
......@@ -59,6 +59,8 @@ struct xsk_umem {
int fd;
int refcount;
struct list_head ctx_list;
bool rx_ring_setup_done;
bool tx_ring_setup_done;
};
struct xsk_ctx {
......@@ -743,26 +745,30 @@ static struct xsk_ctx *xsk_get_ctx(struct xsk_umem *umem, int ifindex,
return NULL;
}
static void xsk_put_ctx(struct xsk_ctx *ctx)
static void xsk_put_ctx(struct xsk_ctx *ctx, bool unmap)
{
struct xsk_umem *umem = ctx->umem;
struct xdp_mmap_offsets off;
int err;
if (--ctx->refcount == 0) {
if (--ctx->refcount)
return;
if (!unmap)
goto out_free;
err = xsk_get_mmap_offsets(umem->fd, &off);
if (!err) {
munmap(ctx->fill->ring - off.fr.desc,
off.fr.desc + umem->config.fill_size *
if (err)
goto out_free;
munmap(ctx->fill->ring - off.fr.desc, off.fr.desc + umem->config.fill_size *
sizeof(__u64));
munmap(ctx->comp->ring - off.cr.desc,
off.cr.desc + umem->config.comp_size *
munmap(ctx->comp->ring - off.cr.desc, off.cr.desc + umem->config.comp_size *
sizeof(__u64));
}
out_free:
list_del(&ctx->list);
free(ctx);
}
}
static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
......@@ -797,8 +803,6 @@ static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
memcpy(ctx->ifname, ifname, IFNAMSIZ - 1);
ctx->ifname[IFNAMSIZ - 1] = '\0';
umem->fill_save = NULL;
umem->comp_save = NULL;
ctx->fill = fill;
ctx->comp = comp;
list_add(&ctx->list, &umem->ctx_list);
......@@ -854,6 +858,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
struct xsk_socket *xsk;
struct xsk_ctx *ctx;
int err, ifindex;
bool unmap = umem->fill_save != fill;
bool rx_setup_done = false, tx_setup_done = false;
if (!umem || !xsk_ptr || !(rx || tx))
return -EFAULT;
......@@ -881,6 +887,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
}
} else {
xsk->fd = umem->fd;
rx_setup_done = umem->rx_ring_setup_done;
tx_setup_done = umem->tx_ring_setup_done;
}
ctx = xsk_get_ctx(umem, ifindex, queue_id);
......@@ -899,7 +907,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
}
xsk->ctx = ctx;
if (rx) {
if (rx && !rx_setup_done) {
err = setsockopt(xsk->fd, SOL_XDP, XDP_RX_RING,
&xsk->config.rx_size,
sizeof(xsk->config.rx_size));
......@@ -907,8 +915,10 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
err = -errno;
goto out_put_ctx;
}
if (xsk->fd == umem->fd)
umem->rx_ring_setup_done = true;
}
if (tx) {
if (tx && !tx_setup_done) {
err = setsockopt(xsk->fd, SOL_XDP, XDP_TX_RING,
&xsk->config.tx_size,
sizeof(xsk->config.tx_size));
......@@ -916,6 +926,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
err = -errno;
goto out_put_ctx;
}
if (xsk->fd == umem->fd)
umem->rx_ring_setup_done = true;
}
err = xsk_get_mmap_offsets(xsk->fd, &off);
......@@ -994,6 +1006,8 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
}
*xsk_ptr = xsk;
umem->fill_save = NULL;
umem->comp_save = NULL;
return 0;
out_mmap_tx:
......@@ -1005,7 +1019,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
munmap(rx_map, off.rx.desc +
xsk->config.rx_size * sizeof(struct xdp_desc));
out_put_ctx:
xsk_put_ctx(ctx);
xsk_put_ctx(ctx, unmap);
out_socket:
if (--umem->refcount)
close(xsk->fd);
......@@ -1019,6 +1033,9 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
struct xsk_ring_cons *rx, struct xsk_ring_prod *tx,
const struct xsk_socket_config *usr_config)
{
if (!umem)
return -EFAULT;
return xsk_socket__create_shared(xsk_ptr, ifname, queue_id, umem,
rx, tx, umem->fill_save,
umem->comp_save, usr_config);
......@@ -1068,7 +1085,7 @@ void xsk_socket__delete(struct xsk_socket *xsk)
}
}
xsk_put_ctx(ctx);
xsk_put_ctx(ctx, true);
umem->refcount--;
/* Do not close an fd that also has an associated umem connected
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment