RFE: mount: define new option string "rro" for MOUNT_ATTR_RDONLY + AT_RECURSIVE

[AkihiroSuda]

Background

So far, mount -o ro,rbind has not been recursively read-only.

Kernel 5.12 added a new syscall mount_setattr(2) for supporting recursively read-only mounts, but this syscall is not currently used by the mount(8) command of util-linux.

struct mount_attr {
	__u64 attr_set;
	__u64 attr_clr;
	__u64 propagation;
	__u64 userns_fd;
};

int mount_setattr(int dfd, const char *path, unsigned flags,
                  struct mount_attr *uattr, size_t usize);

struct mount_attr attr = {
  .attr_set   = MOUNT_ATTR_RDONLY,
};
rc = mount_setattr(-1, "/mnt/ro", AT_RECURSIVE, &attr, sizeof(attr));

RFE

I suggest defining a new mount(8) option string "rro" for supporting MOUNT_ATTR_RDONLY + AT_RECURSIVE.
"rro" is chosen for consistency with other existing option strings like "rprivate" (recursive "private").

The behavior of the existing "ro" option should remain unchaged, for compatibility reason.

[karelzak]

I like the idea, but it's definitely not about ro only. It is also about others VFS flags (noexec, nosuid, etc.). There is also a little bit different semantic as mount_setattr() can also "clear" flags. The classic mount(2) replaces all old flags with a new flags :-(.

I'll try to implement rro in some basic way already for the next release (mount -oremount,rro or mount -orbind,rro).

But, my long-term goal is to move libmount to the new kernel API (open_tree(), move_mount(), fsconfig, mount_setattr(), and explicitly differentiate between set/clear to make it more user-friendly, something like

mount modify --clear noexec --set nodev,private,rro /mnt

[brauner]

I like the idea, but it's definitely not about ro only. It is also about others VFS flags (noexec, nosuid, etc.). There is also a little bit different semantic as mount_setattr() can also "clear" flags. The classic mount(2) replaces all old flags with a new flags :-(.

I'll try to implement rro in some basic way already for the next release (mount -oremount,rro or mount -orbind,rro).

But, my long-term goal is to move libmount to the new kernel API (open_tree(), move_mount(), fsconfig, mount_setattr(), and explicitly differentiate between set/clear to make it more user-friendly, something like

mount modify --clear noexec --set nodev,private,rro /mnt

I'm loving this plan!

[AkihiroSuda]

@karelzak Thanks for your recent work on the AT_RECURSIVE support, but it looks like mount -o rbind,ro now always makes the mount point recursively read-only? (Tested the latest master commit ce2d993 on Ubuntu 22.10)

Isn't this a breaking change?

Potential existing usecases of non-recursive read-only recursive bind mounts would be like:

• Mount /src as RO, while leaving /src/_output writable
• Mount /lecture_materials as RO, while leaving /lecture_materials/student_assignment_submission writable

[karelzak]

@karelzak Thanks for your recent work on the AT_RECURSIVE support, but it looks like mount -o rbind,ro now always makes the mount point recursively read-only?

I guess if you use any flags with recursive (bind) operation, then it's expected that you also want the flag to apply recursively, but ...

... unfortunately, now I see that classic mount(2) interprets MS_REC|MS_REMOUNT|MS_BIND|MS_RDONLY as non-recursive operation, and it sets only first VFS node read-only ;-(((

# strace -e mount  mount -orbind,ro /mnt/A /mnt/B
mount("/mnt/A", "/mnt/B", 0x562e6594fae0, MS_RDONLY|MS_BIND|MS_REC, NULL) = 0
mount("none", "/mnt/B", NULL, MS_RDONLY|MS_REMOUNT|MS_BIND|MS_REC, NULL) = 0

# findmnt
 ─/mnt/A                               /dev/sdc1     ext4      rw,relatime,stripe=512
│ └─/mnt/A/subdir                      tmpfs         tmpfs     rw,relatime,inode64
└─/mnt/B                               /dev/sdc1     ext4      ro,relatime,stripe=512
  └─/mnt/B/subdir                      tmpfs         tmpfs     rw,relatime,inode64

But the same for the current libmount ends with:

`─/mnt/B                               /dev/sdc1     ext4      ro,relatime,stripe=512
  └─/mnt/B/subdir                      tmpfs         tmpfs     ro,relatime,inode64

Anyway, I'm still not sure if introducing a recursive variant for all VFS flags (nosuid, rnosuid, ..) is the right way to go.

BTW, there is also the suggestion from Lennart to use arguments for the flag to specify where to apply the flag (vfs, fs, blkdev). Maybe we can use the same to set it recursive (ro=recursive).

Isn't this a breaking change?

Yes, rbind,ro is not backwardly compatible in the current git tree ;-( I'll fix it.

[karelzak]

Implemented as ro=recursive.