| Message ID | 20210625221612.2637086-1-kuba@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [bpf-next] xdp: move the rxq_info.mem clearing to unreg_mem_model() | expand |
Hello: This patch was applied to bpf/bpf-next.git (refs/heads/master): On Fri, 25 Jun 2021 15:16:12 -0700 you wrote: > 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. > > [...] Here is the summary with links: - [bpf-next] xdp: move the rxq_info.mem clearing to unreg_mem_model() https://git.kernel.org/bpf/bpf-next/c/a78cae247681 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/core/xdp.c b/net/core/xdp.c index 858276e72c68..6ea3302c42eb 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -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; @@ -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); @@ -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);
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> --- Targeting bpf-next, the mvpp2 bug (if it's real) can be addressed separately. IMHO it may be a little late in the cycle for "API convenience" changes but I tested against bpf as well. net/core/xdp.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)