On Tue, Nov 28, 2023 at 05:03:34PM +0100, Miklos Szeredi wrote:
> 1) Make permission checking consistent with statmount(2): fail if mount is
> unreachable from current root. Previously it failed if mount was
> unreachable from root->mnt->mnt_root.
>
> 2) List all submounts, even if unreachable from current root. This is
> safe, since 1) will prevent listing unreachable mounts for unprivileged
> users.
>
> 3) LSMT_ROOT is unchaged, it lists mounts under current root.
>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> ---
> fs/namespace.c | 39 ++++++++++++++-------------------------
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index ad62cf7ee334..10cd651175b5 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -5004,37 +5004,26 @@ static struct mount *listmnt_next(struct mount *curr)
> return node_to_mount(rb_next(&curr->mnt_node));
> }
>
> -static ssize_t do_listmount(struct mount *first, struct vfsmount *mnt,
> +static ssize_t do_listmount(struct mount *first, struct path *orig, u64 mnt_id,
> u64 __user *buf, size_t bufsize,
> const struct path *root)
> {
> - struct mount *r, *m = real_mount(mnt);
> - struct path rootmnt = {
> - .mnt = root->mnt,
> - .dentry = root->mnt->mnt_root
> - };
> - struct path orig;
> + struct mount *r;
> ssize_t ctr;
> int err;
>
> - if (!is_path_reachable(m, mnt->mnt_root, &rootmnt))
> - return capable(CAP_SYS_ADMIN) ? 0 : -EPERM;
> + if (!capable(CAP_SYS_ADMIN) &&
Was there a reason to do the capable check first? In general,
checking capable() when not needed is frowned upon, as it will
set the PF_SUPERPRIV flag.
> + !is_path_reachable(real_mount(orig->mnt), orig->dentry, root))
> + return -EPERM;
>
> - err = security_sb_statfs(mnt->mnt_root);
> + err = security_sb_statfs(orig->dentry);
> if (err)
> return err;
>
> - if (root->mnt == mnt) {
> - orig = *root;
> - } else {
> - orig.mnt = mnt;
> - orig.dentry = mnt->mnt_root;
> - }
> -
> for (ctr = 0, r = first; r; r = listmnt_next(r)) {
> - if (r == m)
> + if (r->mnt_id_unique == mnt_id)
> continue;
> - if (!is_path_reachable(r, r->mnt.mnt_root, &orig))
> + if (!is_path_reachable(r, r->mnt.mnt_root, orig))
> continue;
>
> if (ctr >= bufsize)
> @@ -5053,9 +5042,8 @@ SYSCALL_DEFINE4(listmount, const struct mnt_id_req __user *, req,
> {
> struct mnt_namespace *ns = current->nsproxy->mnt_ns;
> struct mnt_id_req kreq;
> - struct vfsmount *mnt;
> struct mount *first;
> - struct path root;
> + struct path root, orig;
> u64 mnt_id;
> ssize_t ret;
>
> @@ -5071,16 +5059,17 @@ SYSCALL_DEFINE4(listmount, const struct mnt_id_req __user *, req,
> down_read(&namespace_sem);
> get_fs_root(current->fs, &root);
> if (mnt_id == LSMT_ROOT) {
> - mnt = root.mnt;
> + orig = root;
> } else {
> ret = -ENOENT;
> - mnt = lookup_mnt_in_ns(mnt_id, ns);
> - if (!mnt)
> + orig.mnt = lookup_mnt_in_ns(mnt_id, ns);
> + if (!orig.mnt)
> goto err;
> + orig.dentry = orig.mnt->mnt_root;
> }
> first = node_to_mount(rb_first(&ns->mounts));
>
> - ret = do_listmount(first, mnt, buf, bufsize, &root);
> + ret = do_listmount(first, &orig, mnt_id, buf, bufsize, &root);
> err:
> path_put(&root);
> up_read(&namespace_sem);
> --
> 2.41.0
>
