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@vger.kernel.org Signed-off-by: Christian Brauner (Amutable) --- 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