From bb8d25bbfc3cc2d25f48ee337e500b1f50dbec1c Mon Sep 17 00:00:00 2001 From: dujunbao <14829755+du-junbao@user.noreply.gitee.com> Date: Tue, 22 Sep 2026 22:00:39 +0800 Subject: [PATCH] ub: ubmgr: drop the ping tjetty reference and unlink it atomically ping_tjetty_put() decides whether the reference it is dropping is the last one with kref_read() under ctx->tjetty_lock, and then calls kref_put() after dropping the lock: spin_lock_irqsave(&ctx->tjetty_lock, flag); if (kref_read(&entry->kref) == 1) { hlist_del(&entry->node); last = true; } spin_unlock_irqrestore(&ctx->tjetty_lock, flag); kref_put(&entry->kref, __ping_tjetty_free_entry); if (last) kfree(entry); Two references are dropped concurrently on the ping workqueue, for example when the completions of two pings to the same peer are processed at the same time, and both readers can observe a count above one: neither of them unlinks the entry, while the two kref_put() calls still drive the count to zero, which runs the release callback and frees the tjetty. The entry then stays in the hash with a freed tjetty, so the next __ping_tjetty_find() from ping_tjetty_find_or_create() dereferences it, and the entry itself is never freed. Use refcount_dec_and_lock() on the entry reference: the count update and the hash removal then happen together, and the entry can no longer be found once its last reference is gone. The jetty is unimported after the lock is dropped because that call may sleep, so the field becomes a refcount_t and the release callback turns into a plain helper. Fixes: 17b90c3552bd ("urma: fix potential deadlock risk in urma_ping") --- drivers/ub/urma/ubcore/ubmgr/ubmgr_ping.c | 35 ++++++++++------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/drivers/ub/urma/ubcore/ubmgr/ubmgr_ping.c b/drivers/ub/urma/ubcore/ubmgr/ubmgr_ping.c index a8a83c229e74..fdea2a737679 100644 --- a/drivers/ub/urma/ubcore/ubmgr/ubmgr_ping.c +++ b/drivers/ub/urma/ubcore/ubmgr/ubmgr_ping.c @@ -46,7 +46,7 @@ struct ubmgr_ping_ctx { /* Hash func */ struct ubmgr_ping_tjetty_entry { struct ubcore_tjetty *tjetty; - struct kref kref; + refcount_t ref; struct hlist_node node; }; @@ -81,15 +81,12 @@ __ping_tjetty_new_entry(struct ubcore_device *dev, union ubcore_eid *dst_eid, return ERR_PTR(-ENOMEM); entry->tjetty = tjetty; - kref_init(&entry->kref); + refcount_set(&entry->ref, 1); return entry; } -static void __ping_tjetty_free_entry(struct kref *kref) +static void ping_tjetty_free_entry(struct ubmgr_ping_tjetty_entry *entry) { - struct ubmgr_ping_tjetty_entry *entry = - container_of(kref, struct ubmgr_ping_tjetty_entry, kref); - ubcore_unimport_jetty(entry->tjetty); } @@ -103,7 +100,7 @@ __ping_tjetty_find(struct hlist_head *bucket, union ubcore_eid *dst_eid, if (memcmp(&entry->tjetty->cfg.id.eid, dst_eid, sizeof(union ubcore_eid)) == 0 && entry->tjetty->cfg.id.id == remote_id) { - kref_get(&entry->kref); + refcount_inc(&entry->ref); return entry; } } @@ -131,7 +128,7 @@ static void __ping_tjetty_clear(struct hlist_head *bucket) hlist_for_each_entry_safe(entry, tmp, bucket, node) { hlist_del(&entry->node); - __ping_tjetty_free_entry(&entry->kref); + ping_tjetty_free_entry(entry); kfree(entry); } } @@ -170,7 +167,7 @@ ping_tjetty_find_or_create(struct ubmgr_ping_ctx *ctx, if (entry_added != entry) { ubcore_log_info("Tjetty already imported. deid:" EID_FMT ".\n", EID_ARGS(*dst_eid)); - __ping_tjetty_free_entry(&entry->kref); + ping_tjetty_free_entry(entry); kfree(entry); return entry_added; } @@ -181,19 +178,17 @@ ping_tjetty_find_or_create(struct ubmgr_ping_ctx *ctx, static void ping_tjetty_put(struct ubmgr_ping_ctx *ctx, struct ubmgr_ping_tjetty_entry *entry) { - unsigned long flag; - bool last = false; - - spin_lock_irqsave(&ctx->tjetty_lock, flag); - if (kref_read(&entry->kref) == 1) { + /* + * Drop the reference and, when it was the last one, remove the entry + * from the hash before the jetty is unimported: the lookup side holds + * the same lock, so it can no longer find an entry that is going away. + */ + if (refcount_dec_and_lock(&entry->ref, &ctx->tjetty_lock)) { hlist_del(&entry->node); - last = true; - } - spin_unlock_irqrestore(&ctx->tjetty_lock, flag); - - kref_put(&entry->kref, __ping_tjetty_free_entry); - if (last) + spin_unlock(&ctx->tjetty_lock); + ping_tjetty_free_entry(entry); kfree(entry); + } } static void ping_tjetty_clear(struct ubmgr_ping_ctx *ctx) -- Gitee