mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ocfs2: Keep the dentry lock inode alive until callbacks drain
@ 2026-10-15 16:03 Cen Zhang
  2026-10-19 11:20 ` Joseph Qi
  0 siblings, 1 reply; 3+ messages in thread
From: Cen Zhang @ 2026-10-15 16:03 UTC (permalink / raw)
  To: mark, jlbec, joseph.qi, heming.zhao, jack, akpm, srinivas.eeda, rgoldwyn
  Cc: ocfs2-devel, linux-kernel, stable, baijiaju1990, jjzuming, zzzccc427

The inode reference held by a DENTRY lock must outlive callbacks on
that lock. ocfs2_drop_dentry_lock() drops dl_inode before marking the
lock freeing and waiting for DLM unlock completion, although the BAST
path still reads dl_inode->i_sb in ocfs2_get_dentry_osb().

An EX request, such as a remote unlink or rename, can make the
downconvert worker delete all matching aliases. If no other inode
reference remains, its post_unlock callback performs the final iput().
The O2CB DLM can deliver another BAST while the request is still
blocked, for example when another holder releases the same resource:

  Downconvert worker                 DLM callback
  ------------------                 ------------
  Remove dentry aliases
  Clear QUEUED; post_unlock()
  ocfs2_drop_dentry_lock()
    iput(dl->dl_inode)
    [inode reclaimed after RCU]
                                     Duplicate BAST
                                     ocfs2_blocking_ast()
                                       ocfs2_get_dentry_osb()
                                         Read freed dl_inode->i_sb
  ocfs2_simple_drop_lockres()

Call ocfs2_simple_drop_lockres() before iput() so the existing inode
reference remains held while queued work and DLM callbacks are drained.
The inode can only be evicted after the DENTRY lock has been unlocked.

KASAN report as below:

    BUG: KASAN: slab-use-after-free in ocfs2_get_dentry_osb+0x155/0x170
    Read of size 8 at addr ffff8881100ff888 by task dlm-5B530197BEF/575

    CPU: 0 UID: 0 PID: 575 Comm: dlm-5B530197BEF Not tainted 7.3.0-rc4-next-20260921-pmb-ocfs2-functional-v1+ #1 PREEMPT(lazy)
    Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? ocfs2_get_dentry_osb+0x155/0x170
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20e/0x420
     ? ocfs2_get_dentry_osb+0x155/0x170
     kasan_report+0xe0/0x110
     ? ocfs2_get_dentry_osb+0x155/0x170
     ? __pfx_ocfs2_get_dentry_osb+0x10/0x10
     ? __pfx_o2dlm_blocking_ast_wrapper+0x10/0x10
     ocfs2_get_dentry_osb+0x155/0x170
     ocfs2_blocking_ast+0xd3/0x6d0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __lock_acquire+0x5a3/0x2140
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __mlog_printk+0x212/0x2a0
     ? __pfx_ocfs2_blocking_ast+0x10/0x10
     ? __pfx_o2dlm_blocking_ast_wrapper+0x10/0x10
     dlm_do_local_bast+0x20c/0x300
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __pfx_dlm_do_local_bast+0x10/0x10
     ? dlm_thread+0x2e36/0x4480
     dlm_thread+0x32b8/0x4480
     ? __pfx_dlm_thread+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __kthread_parkme+0xc1/0x220
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_release+0xc8/0x290
     ? __pfx_autoremove_wake_function+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __kthread_parkme+0x177/0x220
     ? __pfx_dlm_thread+0x10/0x10
     kthread+0x351/0x460
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __pfx_kthread+0x10/0x10
     ret_from_fork+0x659/0x940
     ? __pfx_ret_from_fork+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __switch_to+0x74f/0xf70
     ? __pfx_kthread+0x10/0x10
     ret_from_fork_asm+0x1a/0x30
     </TASK>

    Allocated by task 584:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_slab_alloc+0x89/0x90
     kmem_cache_alloc_lru_noprof+0x205/0x5c0
     ocfs2_alloc_inode+0x2b/0x100
     alloc_inode+0x69/0x230
     new_inode+0x23/0x1a0
     ocfs2_get_init_inode+0x84/0x190
     ocfs2_mknod+0x8a3/0x2150
     ocfs2_create+0x14f/0x3c0
     lookup_open+0x88f/0x13e0
     path_openat+0x95e/0x2020
     do_file_open+0x21e/0x470
     do_sys_openat2+0xf4/0x1b0
     __x64_sys_openat+0x136/0x1e0
     do_syscall_64+0x114/0x620
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 0:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kmem_cache_free+0x242/0x580
     i_callback+0x4b/0x80
     rcu_core+0x6be/0x1ed0
     handle_softirqs+0x1fa/0x8d0
     __irq_exit_rcu+0x113/0x1e0
     irq_exit_rcu+0xe/0x20
     sysvec_apic_timer_interrupt+0x6c/0x80
     asm_sysvec_apic_timer_interrupt+0x1a/0x20

    Last potentially related work creation:
     kasan_save_stack+0x33/0x60
     kasan_record_aux_stack+0xa7/0xc0
     __call_rcu_common.constprop.0+0x80/0x280
     destroy_inode+0x131/0x1b0
     evict+0x543/0x9f0
     iput+0x680/0xb80
     ocfs2_dentry_lock_put+0xca/0x230
     ocfs2_process_blocked_lock+0x1fb/0x420
     ocfs2_downconvert_thread+0x327/0x730
     kthread+0x351/0x460
     ret_from_fork+0x659/0x940
     ret_from_fork_asm+0x1a/0x30

    The buggy address belongs to the object at ffff8881100ff000
     which belongs to the cache ocfs2_inode_cache of size 3400
    The buggy address is located 2184 bytes inside of
     freed 3400-byte region [ffff8881100ff000, ffff8881100ffd48)

    [page and shadow-memory details omitted]

Fixes: 8ed6b23709b3 ("ocfs2: revert iput deferring code in ocfs2_drop_dentry_lock")
Cc: <stable@vger.kernel.org>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Assisted-by: LLM

---
diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c
index e06774fd89d8..e2b560797f17 100644
--- a/fs/ocfs2/dcache.c
+++ b/fs/ocfs2/dcache.c
@@ -354,8 +354,9 @@ out_attach:
 static void ocfs2_drop_dentry_lock(struct ocfs2_super *osb,
 				   struct ocfs2_dentry_lock *dl)
 {
-	iput(dl->dl_inode);
+	/* Keep the inode alive until DENTRY callbacks have completed. */
 	ocfs2_simple_drop_lockres(osb, &dl->dl_lockres);
+	iput(dl->dl_inode);
 	ocfs2_lock_res_free(&dl->dl_lockres);
 	kfree(dl);
 }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ocfs2: Keep the dentry lock inode alive until callbacks drain
  2026-10-15 16:03 [PATCH] ocfs2: Keep the dentry lock inode alive until callbacks drain Cen Zhang
@ 2026-10-19 11:20 ` Joseph Qi
  2026-10-19 13:44   ` Cen Zhang
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2026-10-19 11:20 UTC (permalink / raw)
  To: Cen Zhang
  Cc: mark, jlbec, heming.zhao, jack, akpm, rgoldwyn, srinivas.eeda,
	ocfs2-devel, linux-kernel, stable, baijiaju1990, jjzuming



On 10/16/26 12:03 AM, Cen Zhang wrote:
> The inode reference held by a DENTRY lock must outlive callbacks on
> that lock. ocfs2_drop_dentry_lock() drops dl_inode before marking the
> lock freeing and waiting for DLM unlock completion, although the BAST
> path still reads dl_inode->i_sb in ocfs2_get_dentry_osb().
> 
> An EX request, such as a remote unlink or rename, can make the
> downconvert worker delete all matching aliases. If no other inode
> reference remains, its post_unlock callback performs the final iput().
> The O2CB DLM can deliver another BAST while the request is still
> blocked, for example when another holder releases the same resource:
> 
>   Downconvert worker                 DLM callback
>   ------------------                 ------------
>   Remove dentry aliases
>   Clear QUEUED; post_unlock()
>   ocfs2_drop_dentry_lock()
>     iput(dl->dl_inode)
>     [inode reclaimed after RCU]
>                                      Duplicate BAST
>                                      ocfs2_blocking_ast()
>                                        ocfs2_get_dentry_osb()
>                                          Read freed dl_inode->i_sb
>   ocfs2_simple_drop_lockres()
> 
> Call ocfs2_simple_drop_lockres() before iput() so the existing inode
> reference remains held while queued work and DLM callbacks are drained.
> The inode can only be evicted after the DENTRY lock has been unlocked.
> 
> KASAN report as below:
> 
>     BUG: KASAN: slab-use-after-free in ocfs2_get_dentry_osb+0x155/0x170
>     Read of size 8 at addr ffff8881100ff888 by task dlm-5B530197BEF/575
> 
>     CPU: 0 UID: 0 PID: 575 Comm: dlm-5B530197BEF Not tainted 7.3.0-rc4-next-20260921-pmb-ocfs2-functional-v1+ #1 PREEMPT(lazy)
>     Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
>     Call Trace:
>      <TASK>
>      dump_stack_lvl+0x93/0xd0
>      print_report+0xce/0x630
>      ? ocfs2_get_dentry_osb+0x155/0x170
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __virt_addr_valid+0x20e/0x420
>      ? ocfs2_get_dentry_osb+0x155/0x170
>      kasan_report+0xe0/0x110
>      ? ocfs2_get_dentry_osb+0x155/0x170
>      ? __pfx_ocfs2_get_dentry_osb+0x10/0x10
>      ? __pfx_o2dlm_blocking_ast_wrapper+0x10/0x10
>      ocfs2_get_dentry_osb+0x155/0x170
>      ocfs2_blocking_ast+0xd3/0x6d0
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __lock_acquire+0x5a3/0x2140
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __mlog_printk+0x212/0x2a0
>      ? __pfx_ocfs2_blocking_ast+0x10/0x10
>      ? __pfx_o2dlm_blocking_ast_wrapper+0x10/0x10
>      dlm_do_local_bast+0x20c/0x300
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __pfx_dlm_do_local_bast+0x10/0x10
>      ? dlm_thread+0x2e36/0x4480
>      dlm_thread+0x32b8/0x4480
>      ? __pfx_dlm_thread+0x10/0x10
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __kthread_parkme+0xc1/0x220
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? lock_release+0xc8/0x290
>      ? __pfx_autoremove_wake_function+0x10/0x10
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __kthread_parkme+0x177/0x220
>      ? __pfx_dlm_thread+0x10/0x10
>      kthread+0x351/0x460
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork+0x659/0x940
>      ? __pfx_ret_from_fork+0x10/0x10
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __switch_to+0x74f/0xf70
>      ? __pfx_kthread+0x10/0x10
>      ret_from_fork_asm+0x1a/0x30
>      </TASK>
> 
>     Allocated by task 584:
>      kasan_save_stack+0x33/0x60
>      kasan_save_track+0x14/0x30
>      __kasan_slab_alloc+0x89/0x90
>      kmem_cache_alloc_lru_noprof+0x205/0x5c0
>      ocfs2_alloc_inode+0x2b/0x100
>      alloc_inode+0x69/0x230
>      new_inode+0x23/0x1a0
>      ocfs2_get_init_inode+0x84/0x190
>      ocfs2_mknod+0x8a3/0x2150
>      ocfs2_create+0x14f/0x3c0
>      lookup_open+0x88f/0x13e0
>      path_openat+0x95e/0x2020
>      do_file_open+0x21e/0x470
>      do_sys_openat2+0xf4/0x1b0
>      __x64_sys_openat+0x136/0x1e0
>      do_syscall_64+0x114/0x620
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
>     Freed by task 0:
>      kasan_save_stack+0x33/0x60
>      kasan_save_track+0x14/0x30
>      kasan_save_free_info+0x3b/0x60
>      __kasan_slab_free+0x5f/0x80
>      kmem_cache_free+0x242/0x580
>      i_callback+0x4b/0x80
>      rcu_core+0x6be/0x1ed0
>      handle_softirqs+0x1fa/0x8d0
>      __irq_exit_rcu+0x113/0x1e0
>      irq_exit_rcu+0xe/0x20
>      sysvec_apic_timer_interrupt+0x6c/0x80
>      asm_sysvec_apic_timer_interrupt+0x1a/0x20
> 
>     Last potentially related work creation:
>      kasan_save_stack+0x33/0x60
>      kasan_record_aux_stack+0xa7/0xc0
>      __call_rcu_common.constprop.0+0x80/0x280
>      destroy_inode+0x131/0x1b0
>      evict+0x543/0x9f0
>      iput+0x680/0xb80
>      ocfs2_dentry_lock_put+0xca/0x230
>      ocfs2_process_blocked_lock+0x1fb/0x420
>      ocfs2_downconvert_thread+0x327/0x730
>      kthread+0x351/0x460
>      ret_from_fork+0x659/0x940
>      ret_from_fork_asm+0x1a/0x30
> 
>     The buggy address belongs to the object at ffff8881100ff000
>      which belongs to the cache ocfs2_inode_cache of size 3400
>     The buggy address is located 2184 bytes inside of
>      freed 3400-byte region [ffff8881100ff000, ffff8881100ffd48)
> 
>     [page and shadow-memory details omitted]
> 
> Fixes: 8ed6b23709b3 ("ocfs2: revert iput deferring code in ocfs2_drop_dentry_lock")

This is a revert for performance. Not sure if the regression will come
back with the new order.

> Cc: <stable@vger.kernel.org>
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> Assisted-by: LLM
> 
> ---
> diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c
> index e06774fd89d8..e2b560797f17 100644
> --- a/fs/ocfs2/dcache.c
> +++ b/fs/ocfs2/dcache.c
> @@ -354,8 +354,9 @@ out_attach:
>  static void ocfs2_drop_dentry_lock(struct ocfs2_super *osb,
>  				   struct ocfs2_dentry_lock *dl)
>  {
> -	iput(dl->dl_inode);
> +	/* Keep the inode alive until DENTRY callbacks have completed. */
>  	ocfs2_simple_drop_lockres(osb, &dl->dl_lockres);
> +	iput(dl->dl_inode);
>  	ocfs2_lock_res_free(&dl->dl_lockres);
>  	kfree(dl);
>  }

Commit 9f70968af3e6e ("ocfs2: Re-order iput in ocfs2_drop_dentry_lock")
is what moved iput() above ocfs2_simple_drop_lockres() to begin with, and
it was a deliberate move:

  Do this to avoid a theoretical (I haven't seen this in practice) race where
  the downconvert thread might drop the dentry lock, allowing a remote unlink
  to proceed before dropping the inode locks. This could bounce access to the
  orphan dir between nodes.

With the new order the downconvert thread gives up the dentry lockres and
only afterwards reaches iput() -> ocfs2_evict_inode() ->
ocfs2_delete_inode(), which takes the nfs_sync lock in shared mode and
then the inode lock exclusive.

Thanks,
Joseph

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ocfs2: Keep the dentry lock inode alive until callbacks drain
  2026-10-19 11:20 ` Joseph Qi
@ 2026-10-19 13:44   ` Cen Zhang
  0 siblings, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-19 13:44 UTC (permalink / raw)
  To: Joseph Qi
  Cc: mark, jlbec, heming.zhao, jack, akpm, rgoldwyn, srinivas.eeda,
	ocfs2-devel, linux-kernel, stable, baijiaju1990, jjzuming

Hi Joseph,

Joseph Qi <joseph.qi@linux.alibaba.com> 于2026年10月19日周一 19:20写道:
>
> Commit 9f70968af3e6e ("ocfs2: Re-order iput in ocfs2_drop_dentry_lock")
> is what moved iput() above ocfs2_simple_drop_lockres() to begin with, and
> it was a deliberate move:
>
>   Do this to avoid a theoretical (I haven't seen this in practice) race where
>   the downconvert thread might drop the dentry lock, allowing a remote unlink
>   to proceed before dropping the inode locks. This could bounce access to the
>   orphan dir between nodes.
>
> With the new order the downconvert thread gives up the dentry lockres and
> only afterwards reaches iput() -> ocfs2_evict_inode() ->
> ocfs2_delete_inode(), which takes the nfs_sync lock in shared mode and
> then the inode lock exclusive.
>
> Thanks,
> Joseph

Thank you for the review and pointing out the history.

I will preserve iput() before ocfs2_simple_drop_lockres(). The stale
access is in ocfs2_get_dentry_osb(),
which obtains osb through dl_inode->i_sb after the final inode
reference may have been dropped.

For v2, I plan to store osb in the lock resource’s existing l_priv
field, use the default osb lookup, and recover
the containing dentry lock with container_of(). The mount context is
freed after DLM shutdown, so the callbacks
can obtain it without accessing the inode.

This preserves the existing inode and open lock release order. I will
send a revised patch soon.

Best regards,
Cen Zhang

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-19 13:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-15 16:03 [PATCH] ocfs2: Keep the dentry lock inode alive until callbacks drain Cen Zhang
2026-10-19 11:20 ` Joseph Qi
2026-10-19 13:44   ` Cen Zhang

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®