Re: [PATCH v3 1/2] ocfs2: snapshot dentry names in file-operation traces

From: Joseph Qi

Date: Wed Oct 21 2026 - 08:05:16 EST

On 10/20/26 11:43 AM, Cen Zhang wrote:
> The open, fsync, read, write and splice-read trace calls pass the live
> dentry name and length to the shared ocfs2__file_ops event class. A
> file reference pins the dentry, but not the external allocation backing
> an individual name. The event needs those bytes when sizing its
> __string field and when copying the string into the trace record.
>
> With the relevant event enabled, a caller can be preempted after
> evaluating its name argument. A concurrent long-to-short rename of
> the same dentry can replace and reclaim the external name before
> the trace callback uses it, provided no other snapshot retains it:
>
> File-operation caller Renamer
> --------------------- -------
> load d_name.name
> <preempted>
> vfs_rename()
> ocfs2_rename()
> ocfs2_dentry_move()
> d_move()/copy_name()
> release rename name snapshot
> RCU reclamation completes
> trace callback sizes/copies
> through the old name pointer
>
> The pointer argument is evaluated before the trace wrapper protects
> probe execution, so that protection does not cover the old name
> across this window. Inline names and separately read lengths can also
> change while the event is being recorded.
>
> Take a dentry name snapshot around each of these five trace calls and

Seems ocfs2_setattr() also risks the same race, and it is missed in last
review comments.

> pass both the string and its length from the snapshot. Inline names
> are copied and external names stay referenced until the trace call
> finishes. Check each event-enabled flag before taking a snapshot so
> disabled tracing does not incur snapshot work. The trace event class
> and its format stay unchanged.
>
> The report below is from the read path; the other four call sites
> have the same unprotected name argument.
>
> KASAN report as below:
>
> BUG: KASAN: slab-use-after-free in strlen+0x85/0xa0
> Read of size 1 at addr ffff88810a5b2b18 by task python3/496
>
> [CPU and host details omitted.]
> Call Trace:
> <TASK>
> dump_stack_lvl+0x93/0xd0
> print_report+0xce/0x630
> ? strlen+0x85/0xa0
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __virt_addr_valid+0x20e/0x420
> ? strlen+0x85/0xa0
> kasan_report+0xe0/0x110
> ? strlen+0x85/0xa0
> strlen+0x85/0xa0
> trace_event_raw_event_ocfs2__file_ops+0xf0/0x480
> ? __pfx_trace_event_raw_event_ocfs2__file_ops+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? pmbd_probe_hit_cookie+0xe7/0x1c0
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __pfx_pmbd_probe_hit_cookie+0x10/0x10
> ocfs2_file_read_iter+0x6cd/0xb30
> ? __pfx_ocfs2_file_read_iter+0x10/0x10
> ? selinux_file_permission+0x11d/0x540
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? security_file_permission+0x26/0x80
> vfs_read+0x7d2/0xc20
> ? __pfx_vfs_read+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __pfx_do_sys_openat2+0x10/0x10
> ksys_read+0x111/0x200
> ? __pfx_ksys_read+0x10/0x10
> ? irqentry_exit+0xbe/0x820
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? trace_hardirqs_on_prepare+0x11d/0x150
> ? srso_alias_return_thunk+0x5/0xfbef5
> do_syscall_64+0x114/0x620
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [User register dump omitted.]
> </TASK>
>
> Allocated by task 489:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x14/0x30
> __kasan_kmalloc+0xaa/0xb0
> __kmalloc_noprof+0x2de/0x780
> __d_alloc+0x5fc/0x8c0
> d_alloc_parallel+0x106/0x11d0
> lookup_open+0x3ff/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
> __rcu_free_sheaf_prepare+0x65/0x2a0
> rcu_free_sheaf_nobarn+0x1d/0x50
> 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
>
> The buggy address belongs to the object at ffff88810a5b2b00
> which belongs to the cache kmalloc-rcl-192 of size 192
> The buggy address is located 24 bytes inside of
> freed 192-byte region [ffff88810a5b2b00, ffff88810a5b2bc0)
>
> [Page and shadow-memory dumps omitted.]
>
> Fixes: 468eedde23d6 ("ocfs2: Remove mlog(0) from fs/ocfs2/file.c")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>
> ---
> fs/ocfs2/file.c | 79 ++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 55 insertions(+), 24 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index d6e977ba6565..3c4637b32c09 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -85,10 +85,17 @@ static int ocfs2_file_open(struct inode *inode, struct file *file)
> int mode = file->f_flags;
> struct ocfs2_inode_info *oi = OCFS2_I(inode);
>
> - trace_ocfs2_file_open(inode, file, file->f_path.dentry,
> - (unsigned long long)oi->ip_blkno,
> - file->f_path.dentry->d_name.len,
> - file->f_path.dentry->d_name.name, mode);
> + if (trace_ocfs2_file_open_enabled()) {
> + struct dentry *dentry = file->f_path.dentry;
> + struct name_snapshot name;
> +
> + take_dentry_name_snapshot(&name, dentry);
> + trace_ocfs2_file_open(inode, file, dentry,
> + oi->ip_blkno,
> + name.name.len, name.name.name,
> + mode);
> + release_dentry_name_snapshot(&name);
> + }
>
> if (file->f_mode & FMODE_WRITE) {
> status = dquot_initialize(inode);
> @@ -174,11 +181,17 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
> tid_t commit_tid;
> bool needs_barrier = false;
>
> - trace_ocfs2_sync_file(inode, file, file->f_path.dentry,
> - oi->ip_blkno,
> - file->f_path.dentry->d_name.len,
> - file->f_path.dentry->d_name.name,
> - (unsigned long long)datasync);
> + if (trace_ocfs2_sync_file_enabled()) {
> + struct dentry *dentry = file->f_path.dentry;
> + struct name_snapshot name;
> +
> + take_dentry_name_snapshot(&name, dentry);
> + trace_ocfs2_sync_file(inode, file, dentry,
> + oi->ip_blkno,
> + name.name.len, name.name.name,
> + (unsigned long long)datasync);
> + release_dentry_name_snapshot(&name);
> + }
>
> if (unlikely(ocfs2_emergency_state(osb)))
> return -EROFS;
> @@ -2392,11 +2405,17 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
> int direct_io = iocb->ki_flags & IOCB_DIRECT ? 1 : 0;
> int nowait = iocb->ki_flags & IOCB_NOWAIT ? 1 : 0;
>
> - trace_ocfs2_file_write_iter(inode, file, file->f_path.dentry,
> - (unsigned long long)OCFS2_I(inode)->ip_blkno,
> - file->f_path.dentry->d_name.len,
> - file->f_path.dentry->d_name.name,
> - (unsigned int)from->nr_segs); /* GRRRRR */
> + if (trace_ocfs2_file_write_iter_enabled()) {
> + struct dentry *dentry = file->f_path.dentry;
> + struct name_snapshot name;
> +
> + take_dentry_name_snapshot(&name, dentry);
> + trace_ocfs2_file_write_iter(inode, file, dentry,
> + OCFS2_I(inode)->ip_blkno,
> + name.name.len, name.name.name,
> + (unsigned int)from->nr_segs);
> + release_dentry_name_snapshot(&name);
> + }
>
> if (!direct_io && nowait)
> return -EOPNOTSUPP;
> @@ -2542,11 +2561,17 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
> int direct_io = iocb->ki_flags & IOCB_DIRECT ? 1 : 0;
> int nowait = iocb->ki_flags & IOCB_NOWAIT ? 1 : 0;
>
> - trace_ocfs2_file_read_iter(inode, filp, filp->f_path.dentry,
> - (unsigned long long)OCFS2_I(inode)->ip_blkno,
> - filp->f_path.dentry->d_name.len,
> - filp->f_path.dentry->d_name.name,
> - to->nr_segs); /* GRRRRR */
> + if (trace_ocfs2_file_read_iter_enabled()) {
> + struct dentry *dentry = filp->f_path.dentry;
> + struct name_snapshot name;
> +
> + take_dentry_name_snapshot(&name, dentry);
> + trace_ocfs2_file_read_iter(inode, filp, dentry,
> + OCFS2_I(inode)->ip_blkno,
> + name.name.len, name.name.name,
> + to->nr_segs);
> + release_dentry_name_snapshot(&name);
> + }
>
>
> if (!inode) {

This looks wried, the inode is dereferenced above in OCFS2_I(inode)->ip_blkno,
but checked below.
file_inode() returns filp->f_inode, so it seems inode is never NULL here.

Thanks,
Joseph