Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp278685pxb; Thu, 21 Jan 2021 07:00:43 -0800 (PST) X-Google-Smtp-Source: ABdhPJyJg+1soAbb8v7YLi9kCLGUT2qmP3g1JCbRjwiC1Q9t08tbhJXDEA0W2vSy7z0LStU7C3Y/ X-Received: by 2002:a17:906:f98f:: with SMTP id li15mr9344537ejb.123.1611241243540; Thu, 21 Jan 2021 07:00:43 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1611241243; cv=none; d=google.com; s=arc-20160816; b=0QM8U7j1ExynTiOAeN0t12YOJTDch8DbGAsWvQwThjG/W7OzklU1FQSItScy9ZMXuy 9RtWbeTBVzYtyfUO+X6vqRY9wHR6e/E/TiXC3K6Xd62gjx26NIkWqGmMynpDGPuRIXkW kT3NRj4poJofbCue3IOB4Rciu7PNV980ko+pob6Yu11KKXjhv0vbnPd3KmjPbPAdNaBq wpq+HGSMFDKv3fPmnw+JXqiagIVzhLEiGK1RHS86sW9zf8ZX/eOhLWDS7xOtzQJY8Sh9 55GZHNDnRMAzFjZyDZmoAgYtNnr515XyM0Zjw2N9+V8N7Hk+dB9muExhxQSct63bh4LT 3mqw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=/HSrCgOl2iJqVxwJYcmXXBKvRNve5minNDZb6dWePrM=; b=mQ0HErc+pkFdAo6iyixAVOiJyEyVM8fQBhdjl7COA9MHoam2tER7VB+4Uu/UjhzKuA 7jbbIYmDe/QV/Yk/ySUvIATn05Zcv5+PFQg/1C0Gf+bFCJrvCSoBwby/NoRmvBYtWNqP Hd9S2lPA8tVQQ5aMX24V1pGkAld2Lthl9lBXBItdQLl2SpHy8cNTeSjTDMDZ7AHdvxGa V5ttVmhtZ15Eechd1obFxso/8Ph7h23+dO63+sdZUvB16kDS7HSoP8DVAy7DwB7IvOTt o3jtzYye4N0WBihaOOVbTarw4CUROSG59+n5SuVsHJfMMnj1+o3ZrhNEecngqqtAELQX XI8A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-ext4-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id zk17si1832010ejb.268.2021.01.21.06.59.54; Thu, 21 Jan 2021 07:00:43 -0800 (PST) Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-ext4-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731965AbhAUNaJ (ORCPT + 99 others); Thu, 21 Jan 2021 08:30:09 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:54822 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731366AbhAUN2r (ORCPT ); Thu, 21 Jan 2021 08:28:47 -0500 Received: from ip5f5af0a0.dynamic.kabel-deutschland.de ([95.90.240.160] helo=wittgenstein.fritz.box) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1l2Zux-0005g7-UT; Thu, 21 Jan 2021 13:22:32 +0000 From: Christian Brauner To: Alexander Viro , Christoph Hellwig , linux-fsdevel@vger.kernel.org Cc: John Johansen , James Morris , Mimi Zohar , Dmitry Kasatkin , Stephen Smalley , Casey Schaufler , Arnd Bergmann , Andreas Dilger , OGAWA Hirofumi , Geoffrey Thomas , Mrunal Patel , Josh Triplett , Andy Lutomirski , Theodore Tso , Alban Crequy , Tycho Andersen , David Howells , James Bottomley , Seth Forshee , =?UTF-8?q?St=C3=A9phane=20Graber?= , Linus Torvalds , Aleksa Sarai , Lennart Poettering , "Eric W. Biederman" , smbarber@chromium.org, Phil Estes , Serge Hallyn , Kees Cook , Todd Kjos , Paul Moore , Jonathan Corbet , containers@lists.linux-foundation.org, linux-security-module@vger.kernel.org, linux-api@vger.kernel.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, linux-integrity@vger.kernel.org, selinux@vger.kernel.org, Christian Brauner Subject: [PATCH v6 31/40] namespace: only take read lock in do_reconfigure_mnt() Date: Thu, 21 Jan 2021 14:19:50 +0100 Message-Id: <20210121131959.646623-32-christian.brauner@ubuntu.com> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210121131959.646623-1-christian.brauner@ubuntu.com> References: <20210121131959.646623-1-christian.brauner@ubuntu.com> MIME-Version: 1.0 X-Patch-Hashes: v=1; h=sha256; i=I7aJzwt0DpkJRJifO843fvdrR3C7hhJVdcz6GAHs5To=; m=kYGZs6uxDqNnDWq6amn5PljPB3N7RA7sHRkNgwMp1lA=; p=O0TWEIwrbg8QrF14GljNCoXe8MA31Ulu2rf6pUWyHAo=; g=f6e4b4a5508c730a9bc5fc9fed06d285573d486d X-Patch-Sig: m=pgp; i=christian.brauner@ubuntu.com; s=0x0x91C61BC06578DCA2; b=iHUEABYKAB0WIQRAhzRXHqcMeLMyaSiRxhvAZXjcogUCYAl9pgAKCRCRxhvAZXjcoheiAQCNq6g R7W31jEDQ1qTDxON63E9c5BpR/hnvmNsehDL4bgEA04lytDQ5sUwwJM8ZlP0XfLmigSZ09W7wckQz f0PzmgI= Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org 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. 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]. 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") Link: https://lore.kernel.org/r/20210112220124.837960-4-christian.brauner@ubuntu.com Cc: David Howells Cc: Al Viro Cc: linux-fsdevel@vger.kernel.org Reviewed-by: Christoph Hellwig Signed-off-by: Christian Brauner --- /* v2 */ unchanged /* v3 */ unchanged /* v4 */ unchanged /* v5 */ unchanged base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837 /* v6 */ unchanged base-commit: 19c329f6808995b142b3966301f217c831e7cf31 --- fs/namespace.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index 5fceb2854395..367f1c7cb6db 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -2527,10 +2527,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; @@ -2580,13 +2576,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.30.0