enforce ->sync_fs is only called for rw superblock

Make sure a superblock really is writeable by checking MS_RDONLY under s_umount. sync_filesystems needed some re-arragement for that, but all but one sync_filesystem caller had the correct locking already so that we could add that check there. cachefiles grew s_umount locking. I've also added a WARN_ON to sync_filesystem to assert this for future callers. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

enforce ->sync_fs is only called for rw superblock
Make sure a superblock really is writeable by checking MS_RDONLY under s_umount. sync_filesystems needed some re-arragement for that, but all but one sync_filesystem caller had the correct locking already so that we could add that check there. cachefiles grew s_umount locking. I've also added a WARN_ON to sync_filesystem to assert this for future callers. Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Christoph Hellwig · Al Viro
1 parent e500475338
Showing 5 changed files with 27 additions and 25 deletions Side-by-side Diff
fs/btrfs/super.c
fs/cachefiles/interface.c
fs/reiserfs/super.c
fs/sync.c
fs/ubifs/super.c
@@ -394,9 +394,6 @@
 	struct btrfs_root *root = btrfs_sb(sb);
 	int ret;
  
-	if (sb->s_flags & MS_RDONLY)
-		return 0;
-
 	if (!wait) {
 		filemap_flush(root->fs_info->btree_inode->i_mapping);
 		return 0;
@@ -354,7 +354,9 @@
 	/* make sure all pages pinned by operations on behalf of the netfs are
 	 * written to disc */
 	cachefiles_begin_secure(cache, &saved_cred);
+	down_read(&cache->mnt->mnt_sb->s_umount);
 	ret = sync_filesystem(cache->mnt->mnt_sb);
+	up_read(&cache->mnt->mnt_sb->s_umount);
 	cachefiles_end_secure(cache, saved_cred);
  
 	if (ret == -EIO)
@@ -64,18 +64,15 @@
  
 static int reiserfs_sync_fs(struct super_block *s, int wait)
 {
-	if (!(s->s_flags & MS_RDONLY)) {
-		struct reiserfs_transaction_handle th;
-		reiserfs_write_lock(s);
-		if (!journal_begin(&th, s, 1))
-			if (!journal_end_sync(&th, s, 1))
-				reiserfs_flush_old_commits(s);
-		s->s_dirt = 0;	/* Even if it's not true.
-				 * We'll loop forever in sync_supers otherwise */
-		reiserfs_write_unlock(s);
-	} else {
-		s->s_dirt = 0;
-	}
+	struct reiserfs_transaction_handle th;
+
+	reiserfs_write_lock(s);
+	if (!journal_begin(&th, s, 1))
+		if (!journal_end_sync(&th, s, 1))
+			reiserfs_flush_old_commits(s);
+	s->s_dirt = 0;	/* Even if it's not true.
+			 * We'll loop forever in sync_supers otherwise */
+	reiserfs_write_unlock(s);
 	return 0;
 }
  
@@ -51,6 +51,18 @@
 {
 	int ret;
  
+	/*
+	 * We need to be protected against the filesystem going from
+	 * r/o to r/w or vice versa.
+	 */
+	WARN_ON(!rwsem_is_locked(&sb->s_umount));
+
+	/*
+	 * No point in syncing out anything if the filesystem is read-only.
+	 */
+	if (sb->s_flags & MS_RDONLY)
+		return 0;
+
 	ret = __sync_filesystem(sb, 0);
 	if (ret < 0)
 		return ret;
  
  
  
  
  
@@ -79,25 +91,22 @@
  
 	mutex_lock(&mutex);		/* Could be down_interruptible */
 	spin_lock(&sb_lock);
-	list_for_each_entry(sb, &super_blocks, s_list) {
-		if (sb->s_flags & MS_RDONLY)
-			continue;
+	list_for_each_entry(sb, &super_blocks, s_list)
 		sb->s_need_sync = 1;
-	}
  
 restart:
 	list_for_each_entry(sb, &super_blocks, s_list) {
 		if (!sb->s_need_sync)
 			continue;
 		sb->s_need_sync = 0;
-		if (sb->s_flags & MS_RDONLY)
-			continue;	/* hm.  Was remounted r/o meanwhile */
 		sb->s_count++;
 		spin_unlock(&sb_lock);
+
 		down_read(&sb->s_umount);
-		if (sb->s_root)
+		if (!(sb->s_flags & MS_RDONLY) && sb->s_root)
 			__sync_filesystem(sb, wait);
 		up_read(&sb->s_umount);
+
 		/* restart only when sb is no longer on the list */
 		spin_lock(&sb_lock);
 		if (__put_super_and_need_restart(sb))
@@ -447,9 +447,6 @@
 	if (!wait)
 		return 0;
  
-	if (sb->s_flags & MS_RDONLY)
-		return 0;
-
 	/*
 	 * VFS calls '->sync_fs()' before synchronizing all dirty inodes and
 	 * pages, so synchronize them first, then commit the journal. Strictly
...	...	@@ -394,9 +394,6 @@
394	394	struct btrfs_root *root = btrfs_sb(sb);
395	395	int ret;
396	396
397		- if (sb->s_flags & MS_RDONLY)
398		- return 0;
399		-
400	397	if (!wait) {
401	398	filemap_flush(root->fs_info->btree_inode->i_mapping);
402	399	return 0;
...	...	@@ -354,7 +354,9 @@
354	354	/* make sure all pages pinned by operations on behalf of the netfs are
355	355	* written to disc */
356	356	cachefiles_begin_secure(cache, &saved_cred);
	357	+ down_read(&cache->mnt->mnt_sb->s_umount);
357	358	ret = sync_filesystem(cache->mnt->mnt_sb);
	359	+ up_read(&cache->mnt->mnt_sb->s_umount);
358	360	cachefiles_end_secure(cache, saved_cred);
359	361
360	362	if (ret == -EIO)
...	...	@@ -64,18 +64,15 @@
64	64
65	65	static int reiserfs_sync_fs(struct super_block *s, int wait)
66	66	{
67		- if (!(s->s_flags & MS_RDONLY)) {
68		- struct reiserfs_transaction_handle th;
69		- reiserfs_write_lock(s);
70		- if (!journal_begin(&th, s, 1))
71		- if (!journal_end_sync(&th, s, 1))
72		- reiserfs_flush_old_commits(s);
73		- s->s_dirt = 0; /* Even if it's not true.
74		- * We'll loop forever in sync_supers otherwise */
75		- reiserfs_write_unlock(s);
76		- } else {
77		- s->s_dirt = 0;
78		- }
	67	+ struct reiserfs_transaction_handle th;
	68	+
	69	+ reiserfs_write_lock(s);
	70	+ if (!journal_begin(&th, s, 1))
	71	+ if (!journal_end_sync(&th, s, 1))
	72	+ reiserfs_flush_old_commits(s);
	73	+ s->s_dirt = 0; /* Even if it's not true.
	74	+ * We'll loop forever in sync_supers otherwise */
	75	+ reiserfs_write_unlock(s);
79	76	return 0;
80	77	}
81	78
...	...	@@ -51,6 +51,18 @@
51	51	{
52	52	int ret;
53	53
	54	+ /*
	55	+ * We need to be protected against the filesystem going from
	56	+ * r/o to r/w or vice versa.
	57	+ */
	58	+ WARN_ON(!rwsem_is_locked(&sb->s_umount));
	59	+
	60	+ /*
	61	+ * No point in syncing out anything if the filesystem is read-only.
	62	+ */
	63	+ if (sb->s_flags & MS_RDONLY)
	64	+ return 0;
	65	+
54	66	ret = __sync_filesystem(sb, 0);
55	67	if (ret < 0)
56	68	return ret;
57	69
58	70
59	71
60	72
61	73
...	...	@@ -79,25 +91,22 @@
79	91
80	92	mutex_lock(&mutex); /* Could be down_interruptible */
81	93	spin_lock(&sb_lock);
82		- list_for_each_entry(sb, &super_blocks, s_list) {
83		- if (sb->s_flags & MS_RDONLY)
84		- continue;
	94	+ list_for_each_entry(sb, &super_blocks, s_list)
85	95	sb->s_need_sync = 1;
86		- }
87	96
88	97	restart:
89	98	list_for_each_entry(sb, &super_blocks, s_list) {
90	99	if (!sb->s_need_sync)
91	100	continue;
92	101	sb->s_need_sync = 0;
93		- if (sb->s_flags & MS_RDONLY)
94		- continue; /* hm. Was remounted r/o meanwhile */
95	102	sb->s_count++;
96	103	spin_unlock(&sb_lock);
	104	+
97	105	down_read(&sb->s_umount);
98		- if (sb->s_root)
	106	+ if (!(sb->s_flags & MS_RDONLY) && sb->s_root)
99	107	__sync_filesystem(sb, wait);
100	108	up_read(&sb->s_umount);
	109	+
101	110	/* restart only when sb is no longer on the list */
102	111	spin_lock(&sb_lock);
103	112	if (__put_super_and_need_restart(sb))
...	...	@@ -447,9 +447,6 @@
447	447	if (!wait)
448	448	return 0;
449	449
450		- if (sb->s_flags & MS_RDONLY)
451		- return 0;
452		-
453	450	/*
454	451	* VFS calls '->sync_fs()' before synchronizing all dirty inodes and
455	452	* pages, so synchronize them first, then commit the journal. Strictly