summaryrefslogtreecommitdiff
path: root/fs/namespace.c
diff options
context:
space:
mode:
authorChristian Brauner <brauner@kernel.org>2022-02-03 16:14:11 +0300
committerChristian Brauner <brauner@kernel.org>2022-02-14 10:37:40 +0300
commit87bb5b60019c60e1f902e6885734cc4e5135c2d9 (patch)
tree0617901debd5f10e521856457ceb6e943a6a9d4a /fs/namespace.c
parentad1844a0127af8fbb87d3d7019907260daf6466b (diff)
downloadlinux-87bb5b60019c60e1f902e6885734cc4e5135c2d9.tar.xz
fs: clean up mount_setattr control flow
Simplify the control flow of mount_setattr_{prepare,commit} so they become easier to follow. We kept using both an integer error variable that was passed by pointer as well as a pointer as an indicator for whether or not we need to revert or commit the prepared changes. Simplify this and just use the pointer. If we successfully changed properties the revert pointer will be NULL and if we failed to change properties it will indicate where we failed and thus need to stop reverting. Link: https://lore.kernel.org/r/20220203131411.3093040-8-brauner@kernel.org Cc: Seth Forshee <seth.forshee@digitalocean.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: linux-fsdevel@vger.kernel.org Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Christian Brauner <brauner@kernel.org>
Diffstat (limited to 'fs/namespace.c')
-rw-r--r--fs/namespace.c84
1 files changed, 43 insertions, 41 deletions
diff --git a/fs/namespace.c b/fs/namespace.c
index 00762f9a736a..0e342e2ade83 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -82,6 +82,7 @@ struct mount_kattr {
unsigned int lookup_flags;
bool recurse;
struct user_namespace *mnt_userns;
+ struct mount *revert;
};
/* /sys/fs */
@@ -4011,46 +4012,34 @@ static inline bool mnt_allow_writers(const struct mount_kattr *kattr,
(mnt->mnt.mnt_flags & MNT_READONLY);
}
-static struct mount *mount_setattr_prepare(struct mount_kattr *kattr,
- struct mount *mnt, int *err)
+static int mount_setattr_prepare(struct mount_kattr *kattr, struct mount *mnt)
{
- struct mount *m = mnt, *last = NULL;
-
- if (!is_mounted(&m->mnt)) {
- *err = -EINVAL;
- goto out;
- }
-
- if (!(mnt_has_parent(m) ? check_mnt(m) : is_anon_ns(m->mnt_ns))) {
- *err = -EINVAL;
- goto out;
- }
+ struct mount *m = mnt;
do {
+ int err = -EPERM;
unsigned int flags;
- flags = recalc_flags(kattr, m);
- if (!can_change_locked_flags(m, flags)) {
- *err = -EPERM;
- goto out;
- }
+ kattr->revert = m;
- *err = can_idmap_mount(kattr, m);
- if (*err)
- goto out;
+ flags = recalc_flags(kattr, m);
+ if (!can_change_locked_flags(m, flags))
+ return err;
- last = m;
+ err = can_idmap_mount(kattr, m);
+ if (err)
+ return err;
if (mnt_allow_writers(kattr, m))
continue;
- *err = mnt_hold_writers(m);
- if (*err)
- goto out;
+ err = mnt_hold_writers(m);
+ if (err)
+ return err;
} while (kattr->recurse && (m = next_mnt(m, mnt)));
-out:
- return last;
+ kattr->revert = NULL;
+ return 0;
}
static void do_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
@@ -4078,14 +4067,12 @@ static void do_idmap_mount(const struct mount_kattr *kattr, struct mount *mnt)
put_user_ns(old_mnt_userns);
}
-static void mount_setattr_commit(struct mount_kattr *kattr,
- struct mount *mnt, struct mount *last,
- int err)
+static void mount_setattr_finish(struct mount_kattr *kattr, struct mount *mnt)
{
struct mount *m = mnt;
do {
- if (!err) {
+ if (!kattr->revert) {
unsigned int flags;
do_idmap_mount(kattr, m);
@@ -4097,24 +4084,24 @@ static void mount_setattr_commit(struct mount_kattr *kattr,
if (m->mnt.mnt_flags & MNT_WRITE_HOLD)
mnt_unhold_writers(m);
- if (!err && kattr->propagation)
+ if (!kattr->revert && kattr->propagation)
change_mnt_propagation(m, kattr->propagation);
/*
* On failure, only cleanup until we found the first mount
* we failed to handle.
*/
- if (err && m == last)
- break;
+ if (kattr->revert == m)
+ return;
} while (kattr->recurse && (m = next_mnt(m, mnt)));
- if (!err)
+ if (!kattr->revert)
touch_mnt_namespace(mnt->mnt_ns);
}
static int do_mount_setattr(struct path *path, struct mount_kattr *kattr)
{
- struct mount *mnt = real_mount(path->mnt), *last = NULL;
+ struct mount *mnt = real_mount(path->mnt);
int err = 0;
if (path->dentry != mnt->mnt.mnt_root)
@@ -4135,16 +4122,31 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr)
}
}
+ err = -EINVAL;
lock_mount_hash();
+ /* Ensure that this isn't anything purely vfs internal. */
+ if (!is_mounted(&mnt->mnt))
+ goto out;
+
/*
- * Get the mount tree in a shape where we can change mount
- * properties without failure.
+ * 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.
*/
- last = mount_setattr_prepare(kattr, mnt, &err);
- if (last) /* Commit all changes or revert to the old state. */
- mount_setattr_commit(kattr, mnt, last, err);
+ if (!(mnt_has_parent(mnt) ? check_mnt(mnt) : is_anon_ns(mnt->mnt_ns)))
+ goto out;
+ /*
+ * First, we get the mount tree in a shape where we can change mount
+ * properties without failure. If we succeeded to do so we commit all
+ * changes and if we failed we clean up.
+ */
+ err = mount_setattr_prepare(kattr, mnt);
+ mount_setattr_finish(kattr, mnt);
+
+out:
unlock_mount_hash();
if (kattr->propagation) {