diff options
| author | Sishuai Gong <sishuai.system@gmail.com> | 2023-08-17 20:00:31 -0400 |
|---|---|---|
| committer | Steven Rostedt (Google) <rostedt@goodmis.org> | 2023-08-22 05:23:53 -0400 |
| commit | 086629773ec96216d06c72c801602cc56ebece27 (patch) | |
| tree | 1265fa48fc3498d5af895092d6d944dac77e8407 /fs/tracefs | |
| parent | a943188dabf78585fc5ddae0803b8b2c94f1c824 (diff) | |
tracefs: Avoid changing i_mode to a temp value
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>
Diffstat (limited to 'fs/tracefs')
| -rw-r--r-- | fs/tracefs/inode.c | 6 |
1 files 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)) |