diff mbox series

common: Chown mount even if already idmapped to account for remounts

Message ID 20230203175437.30687-1-gniebler@suse.com (mailing list archive)
State New, archived
Headers show
Series common: Chown mount even if already idmapped to account for remounts | expand

Commit Message

Gabriel Niebler Feb. 3, 2023, 5:54 p.m. UTC
This is a logical consequence of introducing the chown check in _idmapped_mount,
since now a read-only mount can be made idmapped successfully. But if the mount
is then remounted rw the chown never happens, as _idmapped_mount sees that it's
already idmapped and bows out early.

This patch fixes that by simply moving the chown ahead of the idmapped check,
so it will be performed in any case, even on already idmapped mounts.

Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
 common/rc | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Christian Brauner Feb. 6, 2023, 2:53 p.m. UTC | #1
On Fri, Feb 03, 2023 at 06:54:37PM +0100, Gabriel Niebler wrote:
> This is a logical consequence of introducing the chown check in _idmapped_mount,
> since now a read-only mount can be made idmapped successfully. But if the mount
> is then remounted rw the chown never happens, as _idmapped_mount sees that it's
> already idmapped and bows out early.
> 
> This patch fixes that by simply moving the chown ahead of the idmapped check,
> so it will be performed in any case, even on already idmapped mounts.
> 
> Signed-off-by: Gabriel Niebler <gniebler@suse.com>
> ---

Looks good to me,
Reviewed-by: Christian Brauner <brauner@kernel.org>

Thanks!
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 19ab8062..88da356e 100644
--- a/common/rc
+++ b/common/rc
@@ -408,10 +408,6 @@  _idmapped_mount()
 	local tmp=`mktemp -d`
 
 	local mount_rec=`findmnt -rncv -S $dev -o OPTIONS`
-	if [[ "$mount_rec" == *"idmapped"* ]]; then
-		return 0
-	fi
-
 	# We create an idmapped mount where {g,u}id 0 writes to disk as
 	# {g,u}id 10000000 and $(id -u fsgqa) + 10000000. We change ownership
 	# of $mnt, provided it's not read-only, so {g,u} id 0 can actually
@@ -419,6 +415,11 @@  _idmapped_mount()
 	if [[ "$mount_rec" != *"ro,"* && "$mount_rec" != *",ro"* ]]; then
 		chown 10000000:10000000 $mnt || return 1
 	fi
+	# But if the mount is already idmapped, then there's nothing more to do.
+	if [[ "$mount_rec" == *"idmapped"* ]]; then
+		return 0
+	fi
+
 	$here/src/vfs/mount-idmapped \
 		--map-mount b:10000000:0:100000000000 \
 		$mnt $tmp