Re: [PATCH v3 2/2] ocfs2: snapshot the dentry name while tracing file release

[Joseph Qi]

On 10/20/26 11:43 AM, Cen Zhang wrote:
> ocfs2_file_release() passes the live dentry name and length to a
> trace event while holding oi->ip_lock. That lock protects the open
> count, but it does not serialize dentry name changes during rename.
> The ocfs2__file_ops event sizes and copies its __string separately,
> so a concurrently rewritten inline name need not remain consistent

This call site can reach the external name case too, which is the same
UAF the PATCH 1 of the series describes.

Thanks,
Joseph

> between those operations. The separately loaded length may also
> describe a different name.
> 
>   Release trace                        Renamer
>   -------------                        -------
>   hold oi->ip_lock
>   size the inline name
>                                        rewrite the inline name
>                                        under dentry->d_lock
>   copy the changed name
> 
> 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 is paired even if tracing is toggled meanwhile.
> The snapshot keeps the trace name stable without extending the
> inode lock over snapshot acquisition.
> 
> 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 | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 3c4637b32c09..8da73d0707ce 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -141,18 +141,26 @@ static int ocfs2_file_open(struct inode *inode, struct file *file)
>  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;