Fix remounting snapshots read-write

It's not enough to preserve/restore MS_RDONLY on the superblock flags
to avoid remounting a snapshot read-write: be explicit about our
intentions to the VFS layer so the readonly bit is updated correctly
in do_remount_sb().

Reviewed-by: Chunwei Chen <tuxoko@gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #6510
Closes #6515

module/zfs/zfs_vfsops.c

@@ -1761,8 +1761,15 @@ zfs_remount(struct super_block *sb, int *flags, zfs_mnt_t *zm)
 {
 	zfsvfs_t *zfsvfs = sb->s_fs_info;
 	vfs_t *vfsp;
+	boolean_t issnap = dmu_objset_is_snapshot(zfsvfs->z_os);
 	int error;
 
+	if ((issnap || !spa_writeable(dmu_objset_spa(zfsvfs->z_os))) &&
+	    !(*flags & MS_RDONLY)) {
+		*flags |= MS_RDONLY;
+		return (EROFS);
+	}
+
 	error = zfsvfs_parse_options(zm->mnt_data, &vfsp);
 	if (error)
 		return (error);
@@ -1772,7 +1779,8 @@ zfs_remount(struct super_block *sb, int *flags, zfs_mnt_t *zm)
 
 	vfsp->vfs_data = zfsvfs;
 	zfsvfs->z_vfs = vfsp;
-	(void) zfs_register_callbacks(vfsp);
+	if (!issnap)
+		(void) zfs_register_callbacks(vfsp);
 
 	return (error);
 }

tests/runfiles/linux.run

@@ -108,7 +108,8 @@ tests = ['zfs_inherit_001_neg', 'zfs_inherit_002_neg', 'zfs_inherit_003_pos']
 tests = ['zfs_mount_001_pos', 'zfs_mount_002_pos', 'zfs_mount_003_pos',
     'zfs_mount_004_pos', 'zfs_mount_005_pos', 'zfs_mount_007_pos',
     'zfs_mount_008_pos', 'zfs_mount_009_neg', 'zfs_mount_010_neg',
-    'zfs_mount_011_neg', 'zfs_mount_012_neg', 'zfs_mount_all_001_pos']
+    'zfs_mount_011_neg', 'zfs_mount_012_neg', 'zfs_mount_all_001_pos',
+    'zfs_mount_remount']
 
 [tests/functional/cli_root/zfs_promote]
 tests = ['zfs_promote_001_pos', 'zfs_promote_002_pos', 'zfs_promote_003_pos',

tests/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile.am

@@ -16,4 +16,5 @@ dist_pkgdata_SCRIPTS = \
 	zfs_mount_010_neg.ksh \
 	zfs_mount_011_neg.ksh \
 	zfs_mount_012_neg.ksh \
-	zfs_mount_all_001_pos.ksh
+	zfs_mount_all_001_pos.ksh \
+	zfs_mount_remount.ksh

tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_remount.ksh

@@ -0,0 +1,144 @@
+#!/bin/ksh -p
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
+# or http://www.opensolaris.org/os/licensing.
+# See the License for the specific language governing permissions
+# and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright 2017, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/cli_root/zfs_mount/zfs_mount.kshlib
+
+#
+# DESCRIPTION:
+# Verify remount functionality, expecially on readonly objects.
+#
+# STRATEGY:
+# 1. Prepare a filesystem and a snapshot
+# 2. Verify we can (re)mount the dataset readonly/read-write
+# 3. Verify we can mount the snapshot and it's mounted readonly
+# 4. Verify we can't remount it read-write
+# 5. Re-import the pool readonly
+# 6. Verify we can't remount its filesystem read-write
+#
+
+verify_runnable "both"
+
+function cleanup
+{
+	log_must zpool export $TESTPOOL
+	log_must zpool import $TESTPOOL
+	snapexists $TESTSNAP && log_must zfs destroy $TESTSNAP
+	[[ -d $MNTPSNAP ]] && log_must rmdir $MNTPSNAP
+	return 0
+}
+
+#
+# Verify the $filesystem is mounted readonly
+# This is preferred over "log_mustnot touch $fs" because we actually want to
+# verify the error returned is EROFS
+#
+function readonlyfs # filesystem
+{
+	typeset filesystem="$1"
+
+	file_write -o create -f $filesystem/file.dat
+	ret=$?
+	if [[ $ret != 30 ]]; then
+		log_fail "Writing to $filesystem did not return EROFS ($ret)."
+	fi
+}
+
+#
+# Verify $dataset is mounted with $option
+#
+function checkmount # dataset option
+{
+	typeset dataset="$1"
+	typeset option="$2"
+
+	options="$(awk -v ds="$dataset" '$1 == ds { print $4 }' /proc/mounts)"
+	if [[ "$options" == '' ]]; then
+		log_fail "Dataset $dataset is not mounted"
+	elif [[ ! -z "${options##*$option*}" ]]; then
+		log_fail "Dataset $dataset is not mounted with expected "\
+		    "option $option ($options)"
+	else
+		log_note "Dataset $dataset is mounted with option $option"
+	fi
+}
+
+log_assert "Verify remount functionality on both filesystem and snapshots"
+
+log_onexit cleanup
+
+# 1. Prepare a filesystem and a snapshot
+TESTFS=$TESTPOOL/$TESTFS
+TESTSNAP="$TESTFS@snap"
+datasetexists $TESTFS || log_must zfs create $TESTFS
+snapexists $TESTSNAP || log_must zfs snapshot $TESTSNAP
+log_must zfs set readonly=off $TESTFS
+MNTPFS="$(get_prop mountpoint $TESTFS)"
+MNTPSNAP="$TESTDIR/zfs_snap_mount"
+log_must mkdir -p $MNTPSNAP
+
+# 2. Verify we can (re)mount the dataset readonly/read-write
+log_must touch $MNTPFS/file.dat
+checkmount $TESTFS 'rw'
+log_must mount -o remount,ro $TESTFS $MNTPFS
+readonlyfs $MNTPFS
+checkmount $TESTFS 'ro'
+log_must mount -o remount,rw $TESTFS $MNTPFS
+log_must touch $MNTPFS/file.dat
+checkmount $TESTFS 'rw'
+
+# 3. Verify we can (re)mount the snapshot readonly
+log_must mount -t zfs $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_must mount -o remount,ro $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_must umount $MNTPSNAP
+
+# 4. Verify we can't remount a snapshot read-write
+# The "mount -o rw" command will succeed but the snapshot is mounted readonly.
+# The "mount -o remount,rw" command must fail with an explicit error.
+log_must mount -t zfs -o rw $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_mustnot mount -o remount,rw $TESTSNAP $MNTPSNAP
+readonlyfs $MNTPSNAP
+checkmount $TESTSNAP 'ro'
+log_must umount $MNTPSNAP
+
+# 5. Re-import the pool readonly
+log_must zpool export $TESTPOOL
+log_must zpool import -o readonly=on $TESTPOOL
+
+# 6. Verify we can't remount its filesystem read-write
+readonlyfs $MNTPFS
+checkmount $TESTFS 'ro'
+log_mustnot mount -o remount,rw $MNTPFS
+readonlyfs $MNTPFS
+checkmount $TESTFS 'ro'
+
+log_pass "Both filesystem and snapshots can be remounted correctly."