Skip to content

Commit

Permalink
xdp: Move the rxq_info.mem clearing to unreg_mem_model()
Browse files Browse the repository at this point in the history
xdp_rxq_info_unreg() implicitly calls xdp_rxq_info_unreg_mem_model().
This may well be confusing to the driver authors, and lead to double free
if they call xdp_rxq_info_unreg_mem_model() before xdp_rxq_info_unreg()
(when mem model type == MEM_TYPE_PAGE_POOL).

In fact error path of mvpp2_rxq_init() seems to currently do exactly that.

The double free will result in refcount underflow in page_pool_destroy().
Make the interface a little more programmer friendly by clearing type and
id so that xdp_rxq_info_unreg_mem_model() can be called multiple times.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20210625221612.2637086-1-kuba@kernel.org
  • Loading branch information
Jakub Kicinski authored and Daniel Borkmann committed Jun 28, 2021
1 parent ccff81e commit a78cae2
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions net/core/xdp.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,13 @@ static void mem_allocator_disconnect(void *allocator)
void xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
{
struct xdp_mem_allocator *xa;
int type = xdp_rxq->mem.type;
int id = xdp_rxq->mem.id;

/* Reset mem info to defaults */
xdp_rxq->mem.id = 0;
xdp_rxq->mem.type = 0;

if (xdp_rxq->reg_state != REG_STATE_REGISTERED) {
WARN(1, "Missing register, driver bug");
return;
Expand All @@ -123,7 +128,7 @@ void xdp_rxq_info_unreg_mem_model(struct xdp_rxq_info *xdp_rxq)
if (id == 0)
return;

if (xdp_rxq->mem.type == MEM_TYPE_PAGE_POOL) {
if (type == MEM_TYPE_PAGE_POOL) {
rcu_read_lock();
xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
page_pool_destroy(xa->page_pool);
Expand All @@ -144,10 +149,6 @@ void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)

xdp_rxq->reg_state = REG_STATE_UNREGISTERED;
xdp_rxq->dev = NULL;

/* Reset mem info to defaults */
xdp_rxq->mem.id = 0;
xdp_rxq->mem.type = 0;
}
EXPORT_SYMBOL_GPL(xdp_rxq_info_unreg);

Expand Down

0 comments on commit a78cae2

Please sign in to comment.