All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Sishuai Gong <sishuai.system@gmail.com>
Subject: [for-next][PATCH 13/14] tracefs: Avoid changing i_mode to a temp value
Date: Wed, 23 Aug 2023 22:18:25 -0400	[thread overview]
Message-ID: <20230824021853.022987766@goodmis.org> (raw)
In-Reply-To: 20230824021812.938245293@goodmis.org

From: Sishuai Gong <sishuai.system@gmail.com>

Right now inode->i_mode is updated twice to reach the desired value
in tracefs_apply_options(). Because there is no lock protecting the two
writes, other threads might read the intermediate value of inode->i_mode.

Thread-1			Thread-2
// tracefs_apply_options()	//e.g., acl_permission_check
inode->i_mode &= ~S_IALLUGO;
				unsigned int mode = inode->i_mode;
inode->i_mode |= opts->mode;

I think there is no need to introduce a lock but it is better to
only update inode->i_mode ONCE, so the readers will either see the old
or latest value, rather than an intermediate/temporary value.

Note, the race is not a security concern as the intermediate value is more
locked down than either the start or end version. This is more just to do
the conversion cleanly.

Link: https://lore.kernel.org/linux-trace-kernel/AB5B0A1C-75D9-4E82-A7F0-CF7D0715587B@gmail.com

Signed-off-by: Sishuai Gong <sishuai.system@gmail.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 fs/tracefs/inode.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index bb6de89eb446..c7a10f965602 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -310,6 +310,7 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
 	struct tracefs_fs_info *fsi = sb->s_fs_info;
 	struct inode *inode = d_inode(sb->s_root);
 	struct tracefs_mount_opts *opts = &fsi->mount_opts;
+	umode_t tmp_mode;
 
 	/*
 	 * On remount, only reset mode/uid/gid if they were provided as mount
@@ -317,8 +318,9 @@ static int tracefs_apply_options(struct super_block *sb, bool remount)
 	 */
 
 	if (!remount || opts->opts & BIT(Opt_mode)) {
-		inode->i_mode &= ~S_IALLUGO;
-		inode->i_mode |= opts->mode;
+		tmp_mode = READ_ONCE(inode->i_mode) & ~S_IALLUGO;
+		tmp_mode |= opts->mode;
+		WRITE_ONCE(inode->i_mode, tmp_mode);
 	}
 
 	if (!remount || opts->opts & BIT(Opt_uid))
-- 
2.40.1

  parent reply	other threads:[~2023-08-24  2:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-24  2:18 [for-next][PATCH 00/14] tracing: More updates for 6.6 Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 01/14] tracing/filters: Dynamically allocate filter_pred.regex Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 02/14] tracing/filters: Enable filtering a cpumask field by another cpumask Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 03/14] tracing/filters: Enable filtering a scalar field by a cpumask Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 04/14] tracing/filters: Enable filtering the CPU common " Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 05/14] tracing/filters: Optimise cpumask vs cpumask filtering when user mask is a single CPU Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 06/14] tracing/filters: Optimise scalar vs cpumask filtering when the " Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 07/14] tracing/filters: Optimise CPU " Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 08/14] tracing/filters: Further optimise scalar vs cpumask comparison Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 09/14] tracing/filters: Document cpumask filtering Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 10/14] tracing: Remove unused function declarations Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 11/14] ftrace: Remove empty declaration ftrace_enable_daemon() and ftrace_disable_daemon() Steven Rostedt
2023-08-24  2:18 ` [for-next][PATCH 12/14] tracing/user_events: Optimize safe list traversals Steven Rostedt
2023-08-24  2:18 ` Steven Rostedt [this message]
2023-08-24  2:18 ` [for-next][PATCH 14/14] tracefs: Remove kerneldoc from struct eventfs_file Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230824021853.022987766@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=sishuai.system@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.