* [PATCH v4 1/2] ocfs2: snapshot dentry names in file and setattr traces
2026-10-22 9:08 [PATCH v4 0/2] ocfs2: stabilize dentry names in file and setattr traces Cen Zhang
@ 2026-10-22 9:08 ` Cen Zhang
2026-10-22 9:08 ` [PATCH v4 2/2] ocfs2: snapshot the dentry name while tracing file release Cen Zhang
1 sibling, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-22 9:08 UTC (permalink / raw)
To: mark, jlbec, joseph.qi
Cc: heming.zhao, ocfs2-devel, linux-kernel, stable, baijiaju1990,
jjzuming, zzzccc427
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.
ocfs2_setattr() passes the same unprotected arguments to its own event.
Both events size and copy a __string field separately. A reference to
the dentry does not pin the external storage backing an individual name.
With an event enabled, a caller can be preempted after loading the name
pointer. A long-to-short rename can then replace the name and reclaim its
old external allocation before the trace callback sizes or copies it,
provided no other snapshot retains that allocation.
The setattr inode lock does not exclude every rename: a same-directory
rename of a source directory need not take that directory's inode lock.
It can therefore change the dentry name while setattr records its event.
Inline names and separately loaded lengths can also change during trace
record sizing and copying.
Take a dentry name snapshot around each of these six trace calls and
pass both the string and length from the snapshot. Check event enablement
before taking a snapshot. Keep the event classes and formats unchanged.
Also remove the redundant NULL-inode check in the read callback.
file_inode() returns the open file's inode, which is already used by the
trace call above the check.
The historical report below is from the read path.
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@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
fs/ocfs2/file.c | 100 +++++++++++++++++++++++++++++++-----------------
1 file changed, 64 insertions(+), 36 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index d6e977ba6565..c6e7606ff9e1 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;
@@ -1132,15 +1145,19 @@ int ocfs2_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
int had_lock;
struct ocfs2_lock_holder oh;
- trace_ocfs2_setattr(inode, dentry,
- (unsigned long long)OCFS2_I(inode)->ip_blkno,
- dentry->d_name.len, dentry->d_name.name,
- attr->ia_valid,
- attr->ia_valid & ATTR_MODE ? attr->ia_mode : 0,
- attr->ia_valid & ATTR_UID ?
+ if (trace_ocfs2_setattr_enabled()) {
+ struct name_snapshot name;
+
+ take_dentry_name_snapshot(&name, dentry);
+ trace_ocfs2_setattr(inode, dentry, OCFS2_I(inode)->ip_blkno,
+ name.name.len, name.name.name, attr->ia_valid,
+ attr->ia_valid & ATTR_MODE ? attr->ia_mode : 0,
+ attr->ia_valid & ATTR_UID ?
from_kuid(&init_user_ns, attr->ia_uid) : 0,
- attr->ia_valid & ATTR_GID ?
+ attr->ia_valid & ATTR_GID ?
from_kgid(&init_user_ns, attr->ia_gid) : 0);
+ release_dentry_name_snapshot(&name);
+ }
status = ocfs2_emergency_state(osb);
if (unlikely(status)) {
@@ -2392,11 +2409,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,17 +2565,16 @@ 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;
- if (!inode) {
- ret = -EINVAL;
- mlog_errno(ret);
- goto bail;
+ 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 (!direct_io && nowait)
@@ -2624,11 +2646,17 @@ static ssize_t ocfs2_file_splice_read(struct file *in, loff_t *ppos,
ssize_t ret = 0;
int lock_level = 0;
- trace_ocfs2_file_splice_read(inode, in, in->f_path.dentry,
- (unsigned long long)OCFS2_I(inode)->ip_blkno,
- in->f_path.dentry->d_name.len,
- in->f_path.dentry->d_name.name,
- flags);
+ if (trace_ocfs2_file_splice_read_enabled()) {
+ struct dentry *dentry = in->f_path.dentry;
+ struct name_snapshot name;
+
+ take_dentry_name_snapshot(&name, dentry);
+ trace_ocfs2_file_splice_read(inode, in, dentry,
+ OCFS2_I(inode)->ip_blkno,
+ name.name.len, name.name.name,
+ flags);
+ release_dentry_name_snapshot(&name);
+ }
/*
* We're fine letting folks race truncates and extending writes with
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v4 2/2] ocfs2: snapshot the dentry name while tracing file release
2026-10-22 9:08 [PATCH v4 0/2] ocfs2: stabilize dentry names in file and setattr traces Cen Zhang
2026-10-22 9:08 ` [PATCH v4 1/2] ocfs2: snapshot " Cen Zhang
@ 2026-10-22 9:08 ` Cen Zhang
1 sibling, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-10-22 9:08 UTC (permalink / raw)
To: mark, jlbec, joseph.qi
Cc: heming.zhao, ocfs2-devel, linux-kernel, stable, baijiaju1990,
jjzuming, zzzccc427
ocfs2_file_release() passes the live dentry name and length to a trace
event while holding oi->ip_lock. This lock protects the open count and
delays RCU reclamation of external names, but it does not serialize name
changes during rename. The dentry can switch between inline and external
names, and a separately loaded length can describe a different name.
The ocfs2__file_ops event sizes and copies its __string separately.
A concurrently rewritten inline name need not remain consistent between
those operations. External names can also be selected at this call site;
the existing spinlock critical section protects their allocation lifetime,
while a snapshot additionally pairs a stable name with its length.
Take a name snapshot before acquiring oi->ip_lock and release it after
dropping the lock. Keep the trace call and open-count argument inside
the original critical section. Cache the event-enabled check so the
snapshot stays paired even if tracing is toggled meanwhile.
Fixes: 468eedde23d6 ("ocfs2: Remove mlog(0) from fs/ocfs2/file.c")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
fs/ocfs2/file.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index c6e7606ff9e1..b3a37b9d259a 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -141,18 +141,26 @@ leave:
static int ocfs2_file_release(struct inode *inode, struct file *file)
{
struct ocfs2_inode_info *oi = OCFS2_I(inode);
+ struct dentry *dentry = file->f_path.dentry;
+ struct name_snapshot name;
+ bool trace_enabled = trace_ocfs2_file_release_enabled();
+
+ if (trace_enabled)
+ take_dentry_name_snapshot(&name, dentry);
spin_lock(&oi->ip_lock);
if (!--oi->ip_open_count)
oi->ip_flags &= ~OCFS2_INODE_OPEN_DIRECT;
- trace_ocfs2_file_release(inode, file, file->f_path.dentry,
- oi->ip_blkno,
- file->f_path.dentry->d_name.len,
- file->f_path.dentry->d_name.name,
- oi->ip_open_count);
+ if (trace_enabled)
+ trace_ocfs2_file_release(inode, file, dentry,
+ oi->ip_blkno, name.name.len,
+ name.name.name, oi->ip_open_count);
spin_unlock(&oi->ip_lock);
+ if (trace_enabled)
+ release_dentry_name_snapshot(&name);
+
ocfs2_free_file_private(inode, file);
return 0;
^ permalink raw reply [flat|nested] 3+ messages in thread