lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Dec 2020 00:56:59 +0100 From: Christian Brauner <christian.brauner@...ntu.com> To: Alexander Viro <viro@...iv.linux.org.uk>, Christoph Hellwig <hch@...radead.org>, linux-fsdevel@...r.kernel.org Cc: John Johansen <john.johansen@...onical.com>, James Morris <jmorris@...ei.org>, Mimi Zohar <zohar@...ux.ibm.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>, Stephen Smalley <stephen.smalley.work@...il.com>, Casey Schaufler <casey@...aufler-ca.com>, Arnd Bergmann <arnd@...db.de>, Andreas Dilger <adilger.kernel@...ger.ca>, OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>, Geoffrey Thomas <geofft@...reload.com>, Mrunal Patel <mpatel@...hat.com>, Josh Triplett <josh@...htriplett.org>, Andy Lutomirski <luto@...nel.org>, Theodore Tso <tytso@....edu>, Alban Crequy <alban@...volk.io>, Tycho Andersen <tycho@...ho.ws>, David Howells <dhowells@...hat.com>, James Bottomley <James.Bottomley@...senpartnership.com>, Seth Forshee <seth.forshee@...onical.com>, Stéphane Graber <stgraber@...ntu.com>, Aleksa Sarai <cyphar@...har.com>, Lennart Poettering <lennart@...ttering.net>, "Eric W. Biederman" <ebiederm@...ssion.com>, smbarber@...omium.org, Phil Estes <estesp@...il.com>, Serge Hallyn <serge@...lyn.com>, Kees Cook <keescook@...omium.org>, Todd Kjos <tkjos@...gle.com>, Paul Moore <paul@...l-moore.com>, Jonathan Corbet <corbet@....net>, containers@...ts.linux-foundation.org, linux-security-module@...r.kernel.org, linux-api@...r.kernel.org, linux-ext4@...r.kernel.org, linux-integrity@...r.kernel.org, selinux@...r.kernel.org, Christian Brauner <christian.brauner@...ntu.com>, Christoph Hellwig <hch@....de> Subject: [PATCH v4 03/40] namespace: only take read lock in do_reconfigure_mnt() do_reconfigure_mnt() used to take the down_write(&sb->s_umount) lock which seems unnecessary since we're not changing the superblock. We're only checking whether it is already read-only. Setting other mount attributes is protected by lock_mount_hash() afaict and not by s_umount. So I think the history of down_write(&sb->s_umount) lock being taken when setting mount attributes dates back to the introduction of MNT_READONLY in [2]. Afaict, this introduced the concept of having read-only mounts in contrast to just having a read-only superblock. When it got introduced it was simply plumbed into do_remount() which already took down_write(&sb->s_umount) because it was only used to actually change the superblock before [2]. Afaict, it would've already been possible back then to only use down_read(&sb->s_umount) for MS_BIND | MS_REMOUNT since actual mount options were protected by the vfsmount lock already. But that would've meant special casing the locking for MS_BIND | MS_REMOUNT in do_remount() which people might not have considered worth it. Then in [1] MS_BIND | MS_REMOUNT mount option changes were split out of do_remount() into do_reconfigure_mnt() but the down_write(&sb->s_umount) lock was simply copied over. Now that we have this be a separate helper only take the down_read(&sb->s_umount) lock since we're only interested in checking whether the super block is currently read-only and blocking any writers from changing it. Essentially, checking that the super block is read-only has the advantage that we can avoid having to go into the slowpath and through MNT_WRITE_HOLD and can simply set the read-only flag on the mount in set_mount_attributes(). [1]: commit 43f5e655eff7 ("vfs: Separate changing mount flags full remount") [2]: commit 2e4b7fcd9260 ("[PATCH] r/o bind mounts: honor mount writer counts at remount") Cc: David Howells <dhowells@...hat.com> Cc: Al Viro <viro@...iv.linux.org.uk> Cc: linux-fsdevel@...r.kernel.org Signed-off-by: Christian Brauner <christian.brauner@...ntu.com> Reviewed-by: Christoph Hellwig <hch@....de> --- /* v2 */ unchanged /* v3 */ unchanged /* v4 */ unchanged --- fs/namespace.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 798bbf4f48ad..8497d149ecaa 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2512,10 +2512,6 @@ static int change_mount_ro_state(struct mount *mnt, unsigned int mnt_flags) return 0; } -/* - * Update the user-settable attributes on a mount. The caller must hold - * sb->s_umount for writing. - */ static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags) { mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK; @@ -2565,13 +2561,17 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags) if (!can_change_locked_flags(mnt, mnt_flags)) return -EPERM; - down_write(&sb->s_umount); + /* + * We're only checking whether the superblock is read-only not changing + * it, so only take down_read(&sb->s_umount). + */ + down_read(&sb->s_umount); lock_mount_hash(); ret = change_mount_ro_state(mnt, mnt_flags); if (ret == 0) set_mount_attributes(mnt, mnt_flags); unlock_mount_hash(); - up_write(&sb->s_umount); + up_read(&sb->s_umount); mnt_warn_timestamp_expiry(path, &mnt->mnt); -- 2.29.2
Powered by blists - more mailing lists