Re: [PATCH 03/10] ovl: handle idmapped mounts in ovl_setattr()

On Mon, Jun 15, 2026 at 3:20 PM Christian Brauner <[email protected]> wrote:
>
> Pass the mount's struct mnt_idmap to setattr_prepare() so that the
> permission checks for a chown/chmod performed through an idmapped
> overlay mount are evaluated in the mount's id space.
>
> The ownership requested in @attr is expressed relative to the overlay
> mount idmap.  Before forwarding the change to the upper layer via
> ovl_do_notify_change() - whose notify_change() applies the upper layer
> idmap in turn - rebase ia_vfsuid/ia_vfsgid into the overlay's own id
> space, i.e. the same space as the overlay inode's i_{u,g}id established
> by ovl_copyattr().  Without this rebase the upper layer would interpret
> the caller's mount-relative id as an upper-relative one and store the
> wrong owner on disk, or reject it with -EOVERFLOW.
>
> from_vfsuid() returns INVALID_UID for an id that the overlay mount
> idmap does not map; that invalid id is carried faithfully into the
> forwarded iattr and rejected by the upper notify_change() via
> vfsuid_has_fsmapping(), so no bogus owner can be written.
>
> No functional change until FS_ALLOW_IDMAP is set on ovl_fs_type; until
> then the overlay mount idmap is &nop_mnt_idmap and from_vfsuid() is the
> identity.
>
> Signed-off-by: Christian Brauner (Amutable) <[email protected]>
> ---
>  fs/overlayfs/inode.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index f59db57dfd55..33734ca971e1 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -26,10 +26,18 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>         bool full_copy_up = false;
>         struct dentry *upperdentry;
>
> -       err = setattr_prepare(&nop_mnt_idmap, dentry, attr);
> +       err = setattr_prepare(idmap, dentry, attr);
>         if (err)
>                 return err;
>
> +       /* Rebase ownership from the mount idmap into overlay id space. */
> +       if (attr->ia_valid & ATTR_UID)
> +               attr->ia_vfsuid = VFSUIDT_INIT(from_vfsuid(idmap,
> +                               i_user_ns(d_inode(dentry)), attr->ia_vfsuid));
> +       if (attr->ia_valid & ATTR_GID)
> +               attr->ia_vfsgid = VFSGIDT_INIT(from_vfsgid(idmap,
> +                               i_user_ns(d_inode(dentry)), attr->ia_vfsgid));
> +
>         if (attr->ia_valid & ATTR_SIZE) {
>                 /* Truncate should trigger data copy up as well */
>                 full_copy_up = true;
>

Won't insist on this as part of your work, but would be nice to
take Joanne's patch [1] first and make this change in backing_inode_setattr()

In any case, Joanne, watch out!
Fuse already has FS_ALLOW_IDMAP, so your passthrough inode
series must take this into account - now you have the template here ;)

Thanks,
Amir.

[1] https://lore.kernel.org/fuse-devel/[email protected]/