linux-kernel - Re: [PATCH 3/3] ocfs2: use READ_ONCE for lockless jinode reads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXxBX2Ul-S6nJzG5@casper.infradead.org>
Date: Fri, 30 Jan 2026 05:27:59 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Li Chen <me@...ux.beauty>
Cc: Mark Fasheh <mark@...heh.com>, Joel Becker <jlbec@...lplan.org>,
	Joseph Qi <joseph.qi@...ux.alibaba.com>,
	ocfs2-devel@...ts.linux.dev, linux-kernel@...r.kernel.org,
	Jan Kara <jack@...e.com>
Subject: Re: [PATCH 3/3] ocfs2: use READ_ONCE for lockless jinode reads

On Fri, Jan 30, 2026 at 11:12:32AM +0800, Li Chen wrote:
> ocfs2 journal commit callback reads jbd2_inode dirty range fields without
> holding journal->j_list_lock.
> 
> Use READ_ONCE() for these reads to correct the concurrency assumptions.

I don't think this is the right solution to the problem.  If it is,
there needs to be much better argumentation in the commit message.

As I understand it, jbd2_journal_file_inode() initialises jinode,
then adds it to the t_inode_list, then drops the j_list_lock.  So the
actual problem we need to address is that there's no memory barrier
between the store to i_dirty_start and the list_add().  Once that's
added, there's no need for a READ_ONCE here.

Or have I misunderstood the problem?

> Suggested-by: Jan Kara <jack@...e.com>
> Signed-off-by: Li Chen <me@...ux.beauty>
> ---
>  fs/ocfs2/journal.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 85239807dec7..7032284cdbd6 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -902,8 +902,11 @@ int ocfs2_journal_alloc(struct ocfs2_super *osb)
>  
>  static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode)
>  {
> -	return filemap_fdatawrite_range(jinode->i_vfs_inode->i_mapping,
> -			jinode->i_dirty_start, jinode->i_dirty_end);
> +	struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
> +	loff_t dirty_start = READ_ONCE(jinode->i_dirty_start);
> +	loff_t dirty_end = READ_ONCE(jinode->i_dirty_end);
> +
> +	return filemap_fdatawrite_range(mapping, dirty_start, dirty_end);
>  }
>  
>  int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
> -- 
> 2.52.0
>