[PATCH RFC 07/28] exfat: keep the current mount options on remount

[Christian Brauner]

exfat_init_fs_context() only inherits uid, gid, fmask and dmask from the
mounted superblock for a reconfigure and everything else starts out with
its default. But exfat_reconfigure() refuses to change iocharset,
keep_last_dots, sys_tz, time_offset and allow_utime and returns -EINVAL
if any of them differs. So on an exfat that was mounted with a
non-default value for one of them every remount fails that doesn't
repeat it. That includes emergency remount (sysrq u) which passes no
options at all and reconfigure_super() warns about it:

  forced remount of a exfat fs returned -22
  WARNING: fs/super.c:1133 at reconfigure_super+0x498/0xb20, CPU#3: kworker/3:1/54

The options that can change on remount have the opposite problem. An
option-less remount silently resets them to their defaults. So
errors=continue turns into errors=remount-ro and zero_size_dir and
discard are dropped.

So start a reconfigure from the current options instead, like
gfs2_init_fs_context() does. Then a remount only changes what it
specifies. The iocharset string is the exception. For mount(2)
->init_fs_context() runs without s_umount and a concurrent remount could
free the string while we copy it. And we can't take s_umount there
because emergency remount already holds it. So leave it NULL in the new
context, only compare it in exfat_reconfigure() if the remount names
one, and keep the superblock's string when the options are swapped. All
of that happens under s_umount.

Note that a shutdown turns discard off for good. But the context of a
remount may have been set up before the shutdown and would turn it back
on. So exfat_reconfigure() keeps discard off once the filesystem was
shut down.

The next patch stops exfat from setting SB_RDONLY after errors. An
errored exfat then stays read-write as far as the VFS is concerned and
emergency remount would run into this for it as well.

Reproducer:

  mount -t exfat -o iocharset=iso8859-1 /dev/vdb /mnt
  echo u > /proc/sysrq-trigger    # "forced remount of a exfat fs returned -22"

  mount -t exfat -o errors=continue /dev/vdc /mnt2
  # remount through mount(2) without data, e.g.
  # mount(NULL, "/mnt2", NULL, MS_REMOUNT | MS_RDONLY, NULL)
  grep /mnt2 /proc/mounts          # errors=remount-ro

Fixes: e6fd5d3a4317 ("exfat: support modifying mount options via remount")
Cc: stable at vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner at kernel.org>
---
 fs/exfat/super.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index a9ea36ba2693..71a348d10c26 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -779,14 +779,12 @@ static int exfat_reconfigure(struct fs_context *fc)
 	exfat_clear_volume_dirty(sb);
 	mutex_unlock(&sbi->s_lock);
 
-	if (new_opts->allow_utime == (unsigned short)-1)
-		new_opts->allow_utime = ~new_opts->fs_dmask & 0022;
-
 	/*
 	 * Since the old settings of these mount options are cached in
 	 * inodes or dentries, they cannot be modified dynamically.
 	 */
-	if (strcmp(new_opts->iocharset, cur_opts->iocharset) ||
+	if ((new_opts->iocharset &&
+	     strcmp(new_opts->iocharset, cur_opts->iocharset)) ||
 	    new_opts->keep_last_dots != cur_opts->keep_last_dots ||
 	    new_opts->sys_tz != cur_opts->sys_tz ||
 	    new_opts->time_offset != cur_opts->time_offset ||
@@ -797,6 +795,10 @@ static int exfat_reconfigure(struct fs_context *fc)
 	    new_opts->allow_utime != cur_opts->allow_utime)
 		return -EINVAL;
 
+	/* A shutdown turned discard off for good. */
+	if (exfat_forced_shutdown(sb))
+		new_opts->discard = 0;
+
 	if (new_opts->discard != cur_opts->discard &&
 	    new_opts->discard &&
 	    !bdev_max_discard_sectors(sb->s_bdev)) {
@@ -805,6 +807,11 @@ static int exfat_reconfigure(struct fs_context *fc)
 	}
 
 	swap(*cur_opts, *new_opts);
+	/*
+	 * The iocharset can't change, keep the current string. The new
+	 * options have none if the remount didn't specify one.
+	 */
+	swap(cur_opts->iocharset, new_opts->iocharset);
 
 	return 0;
 }
@@ -831,23 +838,27 @@ static int exfat_init_fs_context(struct fs_context *fc)
 
 	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE && fc->root) {
 		struct super_block *sb = fc->root->d_sb;
-		struct exfat_mount_options *cur_opts = &EXFAT_SB(sb)->options;
 
-		sbi->options.fs_uid = cur_opts->fs_uid;
-		sbi->options.fs_gid = cur_opts->fs_gid;
-		sbi->options.fs_fmask = cur_opts->fs_fmask;
-		sbi->options.fs_dmask = cur_opts->fs_dmask;
+		/*
+		 * Start from the current options so a remount only changes
+		 * what it specifies. exfat_reconfigure() refuses to change
+		 * most of them, so a remount that doesn't repeat them all
+		 * would fail, e.g. emergency remount which passes none. The
+		 * iocharset string stays with the superblock and is only
+		 * compared if the remount names one, see exfat_reconfigure().
+		 */
+		sbi->options = EXFAT_SB(sb)->options;
+		sbi->options.iocharset = NULL;
 	} else {
 		sbi->options.fs_uid = current_uid();
 		sbi->options.fs_gid = current_gid();
 		sbi->options.fs_fmask = current->fs->umask;
 		sbi->options.fs_dmask = current->fs->umask;
+		sbi->options.allow_utime = -1;
+		sbi->options.errors = EXFAT_ERRORS_RO;
+		exfat_set_iocharset(&sbi->options, exfat_default_iocharset);
 	}
 
-	sbi->options.allow_utime = -1;
-	sbi->options.errors = EXFAT_ERRORS_RO;
-	exfat_set_iocharset(&sbi->options, exfat_default_iocharset);
-
 	fc->s_fs_info = sbi;
 	fc->ops = &exfat_context_ops;
 	return 0;

-- 
2.53.0