Skip to content

Commit 09a1b33

Browse files
author
Al Viro
committed
preparations to taking MNT_WRITE_HOLD out of ->mnt_flags
We have an unpleasant wart in accessibility rules for struct mount. There are per-superblock lists of mounts, used by sb_prepare_remount_readonly() to check if any of those is currently claimed for write access and to block further attempts to get write access on those until we are done. As soon as it is attached to a filesystem, mount becomes reachable via that list. Only sb_prepare_remount_readonly() traverses it and it only accesses a few members of struct mount. Unfortunately, ->mnt_flags is one of those and it is modified - MNT_WRITE_HOLD set and then cleared. It is done under mount_lock, so from the locking rules POV everything's fine. However, it has easily overlooked implications - once mount has been attached to a filesystem, it has to be treated as globally visible. In particular, initializing ->mnt_flags *must* be done either prior to that point or under mount_lock. All other members are still private at that point. Life gets simpler if we move that bit (and that's *all* that can get touched by access via this list) out of ->mnt_flags. It's not even hard to do - currently the list is implemented as list_head one, anchored in super_block->s_mounts and linked via mount->mnt_instance. As the first step, switch it to hlist-like open-coded structure - address of the first mount in the set is stored in ->s_mounts and ->mnt_instance replaced with ->mnt_next_for_sb and ->mnt_pprev_for_sb - the former either NULL or pointing to the next mount in set, the latter - address of either ->s_mounts or ->mnt_next_for_sb in the previous element of the set. In the next commit we'll steal the LSB of ->mnt_pprev_for_sb as replacement for MNT_WRITE_HOLD. Reviewed-by: Christian Brauner <[email protected]> Signed-off-by: Al Viro <[email protected]>
1 parent 5d132cf commit 09a1b33

File tree

4 files changed

+36
-13
lines changed

4 files changed

+36
-13
lines changed

fs/mount.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ struct mount {
6464
#endif
6565
struct list_head mnt_mounts; /* list of children, anchored here */
6666
struct list_head mnt_child; /* and going through their mnt_child */
67-
struct list_head mnt_instance; /* mount instance on sb->s_mounts */
67+
struct mount *mnt_next_for_sb; /* the next two fields are hlist_node, */
68+
struct mount * __aligned(1) *mnt_pprev_for_sb;
69+
/* except that LSB of pprev will be stolen */
6870
const char *mnt_devname; /* Name of device e.g. /dev/dsk/hda1 */
6971
struct list_head mnt_list;
7072
struct list_head mnt_expire; /* link in fs-specific expiry list */

fs/namespace.c

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,27 @@ static inline void mnt_unhold_writers(struct mount *mnt)
730730
mnt->mnt.mnt_flags &= ~MNT_WRITE_HOLD;
731731
}
732732

733+
static inline void mnt_del_instance(struct mount *m)
734+
{
735+
struct mount **p = m->mnt_pprev_for_sb;
736+
struct mount *next = m->mnt_next_for_sb;
737+
738+
if (next)
739+
next->mnt_pprev_for_sb = p;
740+
*p = next;
741+
}
742+
743+
static inline void mnt_add_instance(struct mount *m, struct super_block *s)
744+
{
745+
struct mount *first = s->s_mounts;
746+
747+
if (first)
748+
first->mnt_pprev_for_sb = &m->mnt_next_for_sb;
749+
m->mnt_next_for_sb = first;
750+
m->mnt_pprev_for_sb = &s->s_mounts;
751+
s->s_mounts = m;
752+
}
753+
733754
static int mnt_make_readonly(struct mount *mnt)
734755
{
735756
int ret;
@@ -743,17 +764,16 @@ static int mnt_make_readonly(struct mount *mnt)
743764

744765
int sb_prepare_remount_readonly(struct super_block *sb)
745766
{
746-
struct mount *mnt;
747767
int err = 0;
748768

749769
/* Racy optimization. Recheck the counter under MNT_WRITE_HOLD */
750770
if (atomic_long_read(&sb->s_remove_count))
751771
return -EBUSY;
752772

753773
lock_mount_hash();
754-
list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) {
755-
if (!(mnt->mnt.mnt_flags & MNT_READONLY)) {
756-
err = mnt_hold_writers(mnt);
774+
for (struct mount *m = sb->s_mounts; m; m = m->mnt_next_for_sb) {
775+
if (!(m->mnt.mnt_flags & MNT_READONLY)) {
776+
err = mnt_hold_writers(m);
757777
if (err)
758778
break;
759779
}
@@ -763,9 +783,9 @@ int sb_prepare_remount_readonly(struct super_block *sb)
763783

764784
if (!err)
765785
sb_start_ro_state_change(sb);
766-
list_for_each_entry(mnt, &sb->s_mounts, mnt_instance) {
767-
if (mnt->mnt.mnt_flags & MNT_WRITE_HOLD)
768-
mnt->mnt.mnt_flags &= ~MNT_WRITE_HOLD;
786+
for (struct mount *m = sb->s_mounts; m; m = m->mnt_next_for_sb) {
787+
if (m->mnt.mnt_flags & MNT_WRITE_HOLD)
788+
m->mnt.mnt_flags &= ~MNT_WRITE_HOLD;
769789
}
770790
unlock_mount_hash();
771791

@@ -1207,7 +1227,7 @@ static void setup_mnt(struct mount *m, struct dentry *root)
12071227
m->mnt_parent = m;
12081228

12091229
lock_mount_hash();
1210-
list_add_tail(&m->mnt_instance, &s->s_mounts);
1230+
mnt_add_instance(m, s);
12111231
unlock_mount_hash();
12121232
}
12131233

@@ -1425,7 +1445,7 @@ static void mntput_no_expire(struct mount *mnt)
14251445
mnt->mnt.mnt_flags |= MNT_DOOMED;
14261446
rcu_read_unlock();
14271447

1428-
list_del(&mnt->mnt_instance);
1448+
mnt_del_instance(mnt);
14291449
if (unlikely(!list_empty(&mnt->mnt_expire)))
14301450
list_del(&mnt->mnt_expire);
14311451

fs/super.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,6 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
323323
if (!s)
324324
return NULL;
325325

326-
INIT_LIST_HEAD(&s->s_mounts);
327326
s->s_user_ns = get_user_ns(user_ns);
328327
init_rwsem(&s->s_umount);
329328
lockdep_set_class(&s->s_umount, &type->s_umount_key);
@@ -408,7 +407,7 @@ static void __put_super(struct super_block *s)
408407
list_del_init(&s->s_list);
409408
WARN_ON(s->s_dentry_lru.node);
410409
WARN_ON(s->s_inode_lru.node);
411-
WARN_ON(!list_empty(&s->s_mounts));
410+
WARN_ON(s->s_mounts);
412411
call_rcu(&s->rcu, destroy_super_rcu);
413412
}
414413
}

include/linux/fs.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,8 @@ struct sb_writers {
13241324
struct percpu_rw_semaphore rw_sem[SB_FREEZE_LEVELS];
13251325
};
13261326

1327+
struct mount;
1328+
13271329
struct super_block {
13281330
struct list_head s_list; /* Keep this first */
13291331
dev_t s_dev; /* search index; _not_ kdev_t */
@@ -1358,7 +1360,7 @@ struct super_block {
13581360
__u16 s_encoding_flags;
13591361
#endif
13601362
struct hlist_bl_head s_roots; /* alternate root dentries for NFS */
1361-
struct list_head s_mounts; /* list of mounts; _not_ for fs use */
1363+
struct mount *s_mounts; /* list of mounts; _not_ for fs use */
13621364
struct block_device *s_bdev; /* can go away once we use an accessor for @s_bdev_file */
13631365
struct file *s_bdev_file;
13641366
struct backing_dev_info *s_bdi;

0 commit comments

Comments
 (0)