On Thu, Mar 31, 2022 at 01:23:15PM +0200, Christian Brauner wrote:
> In earlier patches we already passed down the relevant upper or lower
> path to ovl_open_realfile(). Now let the open helpers actually take the
> idmapping of the relevant mount into account when checking permissions.
> This is needed to support idmapped base layers with overlay.
>
> Cc: <linux-unionfs@xxxxxxxxxxxxxxx>
> Tested-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx>
> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>
> ---
> /* v2 */
> unchanged
>
> /* v3 */
> unchanged
> ---
> fs/overlayfs/file.c | 7 +++++--
> fs/overlayfs/util.c | 5 +++--
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 656c30bf20a6..7dd44f4e2757 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -42,6 +42,7 @@ static struct file *ovl_open_realfile(const struct file *file,
> {
> struct inode *realinode = d_inode(realpath->dentry);
> struct inode *inode = file_inode(file);
> + struct user_namespace *real_idmap;
> struct file *realfile;
> const struct cred *old_cred;
> int flags = file->f_flags | OVL_OPEN_FLAGS;
> @@ -51,12 +52,14 @@ static struct file *ovl_open_realfile(const struct file *file,
> if (flags & O_APPEND)
> acc_mode |= MAY_APPEND;
>
> +
> old_cred = ovl_override_creds(inode->i_sb);
> - err = inode_permission(&init_user_ns, realinode, MAY_OPEN | acc_mode);
> + real_idmap = mnt_user_ns(realpath->mnt);
> + err = inode_permission(real_idmap, realinode, MAY_OPEN | acc_mode);
Just a minor nit. Should we rename "real_idmap" to say "real_mnt_userns".
Becuase they are not idmaps. They are just pionters to user namespace
associated with that mount. And in all the vfs helpers you have used
the name "mnt_userns" anyway.
I see that downside is that name of the variable becomes longer. But
I feel "real_mnt_userns" bring more clarity while reading code.
Thanks
Vivek
> if (err) {
> realfile = ERR_PTR(err);
> } else {
> - if (!inode_owner_or_capable(&init_user_ns, realinode))
> + if (!inode_owner_or_capable(real_idmap, realinode))
> flags &= ~O_NOATIME;
>
> realfile = open_with_fake_path(&file->f_path, flags, realinode,
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 79fae06ee10a..7dd2e5e6662a 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -523,6 +523,7 @@ bool ovl_is_whiteout(struct dentry *dentry)
> struct file *ovl_path_open(struct path *path, int flags)
> {
> struct inode *inode = d_inode(path->dentry);
> + struct user_namespace *real_idmap = mnt_user_ns(path->mnt);
> int err, acc_mode;
>
> if (flags & ~(O_ACCMODE | O_LARGEFILE))
> @@ -539,12 +540,12 @@ struct file *ovl_path_open(struct path *path, int flags)
> BUG();
> }
>
> - err = inode_permission(&init_user_ns, inode, acc_mode | MAY_OPEN);
> + err = inode_permission(real_idmap, inode, acc_mode | MAY_OPEN);
> if (err)
> return ERR_PTR(err);
>
> /* O_NOATIME is an optimization, don't fail if not permitted */
> - if (inode_owner_or_capable(&init_user_ns, inode))
> + if (inode_owner_or_capable(real_idmap, inode))
> flags |= O_NOATIME;
>
> return dentry_open(path, flags, current_cred());
> --
> 2.32.0
>
