On Tue 06-02-24 11:22:09, Christian Brauner wrote: > When we added mount_setattr() I added additional checks compared to the > legacy do_reconfigure_mnt() and do_change_type() helpers used by regular > mount(2). If that mount had a parent then verify that the caller and the > mount namespace the mount is attached to match and if not make sure that > it's an anonymous mount. > > The real rootfs falls into neither category. It is neither an anoymous > mount because it is obviously attached to the initial mount namespace > but it also obviously doesn't have a parent mount. So that means legacy > mount(2) allows changing mount properties on the real rootfs but > mount_setattr(2) blocks this. I never thought much about this but of > course someone on this planet of earth changes properties on the real > rootfs as can be seen in [1]. > > Since util-linux finally switched to the new mount api in 2.39 not so > long ago it also relies on mount_setattr() and that surfaced this issue > when Fedora 39 finally switched to it. Fix this. > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2256843 > Reported-by: Karel Zak <kzak@xxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx # v5.12+ > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx> I'm not too confident with subtleties of this code but it looks good to me. So feel free to add: Reviewed-by: Jan Kara <jack@xxxxxxx> Honza > --- > fs/namespace.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 437f60e96d40..fb0286920bce 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -4472,10 +4472,15 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr) > /* > * If this is an attached mount make sure it's located in the callers > * mount namespace. If it's not don't let the caller interact with it. > - * If this is a detached mount make sure it has an anonymous mount > - * namespace attached to it, i.e. we've created it via OPEN_TREE_CLONE. > + * > + * If this mount doesn't have a parent it's most often simply a > + * detached mount with an anonymous mount namespace. IOW, something > + * that's simply not attached yet. But there are apparently also users > + * that do change mount properties on the rootfs itself. That obviously > + * neither has a parent nor is it a detached mount so we cannot > + * unconditionally check for detached mounts. > */ > - if (!(mnt_has_parent(mnt) ? check_mnt(mnt) : is_anon_ns(mnt->mnt_ns))) > + if (mnt_has_parent(mnt) && !check_mnt(mnt)) > goto out; > > /* > > --- > base-commit: 2a42e144dd0b62eaf79148394ab057145afbc3c5 > change-id: 20240206-vfs-mount-rootfs-70aff2e3956d > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
