* [PATCH] ocfs2/dlm: hold the lookup reference across convert AST enqueue
@ 2026-10-14 11:23 Joseph Qi
2026-10-14 15:18 ` Heming Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2026-10-14 11:23 UTC (permalink / raw)
To: Andrew Morton, Heming Zhao
Cc: Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel
dlm_convert_lock_handler() drops the reference it took when it found the
lock on res->granted at its leave label, one statement before it calls
dlm_queue_ast(). dlm_free_dead_locks() takes res->spinlock and drops the
list and base references, so a peer death landing in that gap leaves the
handler's own put as the one that frees the lock, and __dlm_queue_ast()
then reads lock->lockres from freed memory.
The handler is registered next to dlm_create_lock_handler(), so a convert
message buffered on a dying peer's socket arrives on the same o2net
receive work that o2net_disconnect_node() does not wait for. The two can
interleave as follows, with the reference counts as they are before this
fix:
Socket data-ready Node-down thread Receive worker
----------------- ---------------- --------------
o2net_data_ready()
select sc under
callback read lock
o2net_disconnect_node()
queue shutdown
begin flush; new color
queue sc_rx_work
in the new color
release callback lock
shutdown socket
flush returns
read buffered convert_lock
dlm_convert_lock_handler()
find lock on res->granted
dlm_lock_get() refs = 3
lock res->spinlock
__dlmconvert_master()
call_ast = 1
unlock res->spinlock
DLM node-down cleanup
lock res->spinlock
dlm_free_dead_locks()
unlink lock
put list ref refs = 2
put base ref refs = 1
unlock res->spinlock
dlm_lock_put() refs = 0
free lock
dlm_queue_ast()
__dlm_queue_ast()
read lock->lockres
(use-after-free)
__dlmconvert_master() only moves the lock between lists, so the count
entering the gap is base, list and lookup. Drop the lookup reference
after the AST is queued instead of before it and the race ends with the
one __dlm_queue_ast() took, which dlm_flush_asts() releases later with
ast_pending cleared and ast_list unlinked. No extra get is needed, and
the normal case ends with the same count as before.
dlmconvert_master() has the same shape but is only reached from dlmlock()
for a lock this node owns, which dlm_free_dead_locks() never matches.
Fixes: 2c5c54aca9d0 ("ocfs2/dlm: Add missing dlm_lock_put()s")
Cc: <stable@vger.kernel.org>
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
fs/ocfs2/dlm/dlmconvert.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/ocfs2/dlm/dlmconvert.c b/fs/ocfs2/dlm/dlmconvert.c
index 450d46eefab32..8a828448ba958 100644
--- a/fs/ocfs2/dlm/dlmconvert.c
+++ b/fs/ocfs2/dlm/dlmconvert.c
@@ -534,15 +534,19 @@ int dlm_convert_lock_handler(struct o2net_msg *msg, u32 len, void *data,
}
leave:
- if (lock)
- dlm_lock_put(lock);
-
/* either queue the ast or release it, if reserved */
if (call_ast)
dlm_queue_ast(dlm, lock);
else if (ast_reserved)
dlm_lockres_release_ast(dlm, res);
+ /*
+ * Keep the lock alive through AST enqueue: a concurrent
+ * dlm_free_dead_locks() drops the list and base references.
+ */
+ if (lock)
+ dlm_lock_put(lock);
+
if (kick_thread)
dlm_kick_thread(dlm, res);
--
2.39.3
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ocfs2/dlm: hold the lookup reference across convert AST enqueue
2026-10-14 11:23 [PATCH] ocfs2/dlm: hold the lookup reference across convert AST enqueue Joseph Qi
@ 2026-10-14 15:18 ` Heming Zhao
2026-10-15 0:56 ` Joseph Qi
0 siblings, 1 reply; 3+ messages in thread
From: Heming Zhao @ 2026-10-14 15:18 UTC (permalink / raw)
To: Joseph Qi
Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel
On Wed, Oct 14, 2026 at 07:23:21PM +0800, Joseph Qi wrote:
> dlm_convert_lock_handler() drops the reference it took when it found the
> lock on res->granted at its leave label, one statement before it calls
> dlm_queue_ast(). dlm_free_dead_locks() takes res->spinlock and drops the
> list and base references, so a peer death landing in that gap leaves the
> handler's own put as the one that frees the lock, and __dlm_queue_ast()
> then reads lock->lockres from freed memory.
>
> The handler is registered next to dlm_create_lock_handler(), so a convert
> message buffered on a dying peer's socket arrives on the same o2net
> receive work that o2net_disconnect_node() does not wait for. The two can
> interleave as follows, with the reference counts as they are before this
> fix:
>
> Socket data-ready Node-down thread Receive worker
> ----------------- ---------------- --------------
> o2net_data_ready()
> select sc under
> callback read lock
> o2net_disconnect_node()
> queue shutdown
> begin flush; new color
> queue sc_rx_work
> in the new color
> release callback lock
> shutdown socket
> flush returns
> read buffered convert_lock
> dlm_convert_lock_handler()
> find lock on res->granted
> dlm_lock_get() refs = 3
> lock res->spinlock
> __dlmconvert_master()
> call_ast = 1
> unlock res->spinlock
> DLM node-down cleanup
> lock res->spinlock
> dlm_free_dead_locks()
> unlink lock
> put list ref refs = 2
> put base ref refs = 1
> unlock res->spinlock
> dlm_lock_put() refs = 0
> free lock
> dlm_queue_ast()
> __dlm_queue_ast()
> read lock->lockres
> (use-after-free)
>
> __dlmconvert_master() only moves the lock between lists, so the count
> entering the gap is base, list and lookup. Drop the lookup reference
> after the AST is queued instead of before it and the race ends with the
> one __dlm_queue_ast() took, which dlm_flush_asts() releases later with
> ast_pending cleared and ast_list unlinked. No extra get is needed, and
> the normal case ends with the same count as before.
>
> dlmconvert_master() has the same shape but is only reached from dlmlock()
> for a lock this node owns, which dlm_free_dead_locks() never matches.
>
> Fixes: 2c5c54aca9d0 ("ocfs2/dlm: Add missing dlm_lock_put()s")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
The code looks good to me.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
btw, how do you find the issue?
> ---
> fs/ocfs2/dlm/dlmconvert.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ocfs2/dlm/dlmconvert.c b/fs/ocfs2/dlm/dlmconvert.c
> index 450d46eefab32..8a828448ba958 100644
> --- a/fs/ocfs2/dlm/dlmconvert.c
> +++ b/fs/ocfs2/dlm/dlmconvert.c
> @@ -534,15 +534,19 @@ int dlm_convert_lock_handler(struct o2net_msg *msg, u32 len, void *data,
> }
>
> leave:
> - if (lock)
> - dlm_lock_put(lock);
> -
> /* either queue the ast or release it, if reserved */
> if (call_ast)
> dlm_queue_ast(dlm, lock);
> else if (ast_reserved)
> dlm_lockres_release_ast(dlm, res);
>
> + /*
> + * Keep the lock alive through AST enqueue: a concurrent
> + * dlm_free_dead_locks() drops the list and base references.
> + */
> + if (lock)
> + dlm_lock_put(lock);
> +
> if (kick_thread)
> dlm_kick_thread(dlm, res);
>
> --
> 2.39.3
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ocfs2/dlm: hold the lookup reference across convert AST enqueue
2026-10-14 15:18 ` Heming Zhao
@ 2026-10-15 0:56 ` Joseph Qi
0 siblings, 0 replies; 3+ messages in thread
From: Joseph Qi @ 2026-10-15 0:56 UTC (permalink / raw)
To: Heming Zhao
Cc: Andrew Morton, Mark Fasheh, Joel Becker, ocfs2-devel, linux-kernel
On 10/14/26 11:18 PM, Heming Zhao wrote:
> On Wed, Oct 14, 2026 at 07:23:21PM +0800, Joseph Qi wrote:
>> dlm_convert_lock_handler() drops the reference it took when it found the
>> lock on res->granted at its leave label, one statement before it calls
>> dlm_queue_ast(). dlm_free_dead_locks() takes res->spinlock and drops the
>> list and base references, so a peer death landing in that gap leaves the
>> handler's own put as the one that frees the lock, and __dlm_queue_ast()
>> then reads lock->lockres from freed memory.
>>
>> The handler is registered next to dlm_create_lock_handler(), so a convert
>> message buffered on a dying peer's socket arrives on the same o2net
>> receive work that o2net_disconnect_node() does not wait for. The two can
>> interleave as follows, with the reference counts as they are before this
>> fix:
>>
>> Socket data-ready Node-down thread Receive worker
>> ----------------- ---------------- --------------
>> o2net_data_ready()
>> select sc under
>> callback read lock
>> o2net_disconnect_node()
>> queue shutdown
>> begin flush; new color
>> queue sc_rx_work
>> in the new color
>> release callback lock
>> shutdown socket
>> flush returns
>> read buffered convert_lock
>> dlm_convert_lock_handler()
>> find lock on res->granted
>> dlm_lock_get() refs = 3
>> lock res->spinlock
>> __dlmconvert_master()
>> call_ast = 1
>> unlock res->spinlock
>> DLM node-down cleanup
>> lock res->spinlock
>> dlm_free_dead_locks()
>> unlink lock
>> put list ref refs = 2
>> put base ref refs = 1
>> unlock res->spinlock
>> dlm_lock_put() refs = 0
>> free lock
>> dlm_queue_ast()
>> __dlm_queue_ast()
>> read lock->lockres
>> (use-after-free)
>>
>> __dlmconvert_master() only moves the lock between lists, so the count
>> entering the gap is base, list and lookup. Drop the lookup reference
>> after the AST is queued instead of before it and the race ends with the
>> one __dlm_queue_ast() took, which dlm_flush_asts() releases later with
>> ast_pending cleared and ast_list unlinked. No extra get is needed, and
>> the normal case ends with the same count as before.
>>
>> dlmconvert_master() has the same shape but is only reached from dlmlock()
>> for a lock this node owns, which dlm_free_dead_locks() never matches.
>>
>> Fixes: 2c5c54aca9d0 ("ocfs2/dlm: Add missing dlm_lock_put()s")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>
> The code looks good to me.
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
>
> btw, how do you find the issue?
>
This is found when I review patch:
ocfs2/dlm: hold a lock reference across grant AST enqueue
Since they address difference flow, so I send it as a separate patch.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-15 0:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-14 11:23 [PATCH] ocfs2/dlm: hold the lookup reference across convert AST enqueue Joseph Qi
2026-10-14 15:18 ` Heming Zhao
2026-10-15 0:56 ` Joseph Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®