Re: [PATCH 1/6] ovl: add prepare_creds_ovl cleanup guard

[Amir Goldstein <amir73il@gmail.com>]

On Fri, Nov 14, 2025 at 11:15 AM Christian Brauner <brauner@kernel.org> wrote:
>
> The current code to override credentials for creation operations is
> pretty difficult to understand. We effectively override the credentials
> twice:
>
> (1) override with the mounter's credentials
> (2) copy the mounts credentials and override the fs{g,u}id with the inode {u,g}id
>
> And then we elide the revert because it would be an idempotent revert.
> That elision doesn't buy us anything anymore though because I've made it
> all work without any reference counting anyway. All it does is mix the
> two credential overrides together.
>
> We can use a cleanup guard to clarify the creation codepaths and make
> them easier to understand.
>
> This just introduces the cleanup guard keeping the patch reviewable.
> We'll convert the caller in follow-up patches and then drop the
> duplicated code.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/overlayfs/dir.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 0030f5a69d22..87f6c5ea6ce0 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -575,6 +575,42 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
>         goto out_dput;
>  }
>
> +static const struct cred *ovl_prepare_creds(struct dentry *dentry, struct inode *inode, umode_t mode)
> +{
> +       int err;
> +
> +       if (WARN_ON_ONCE(current->cred != ovl_creds(dentry->d_sb)))
> +               return ERR_PTR(-EINVAL);
> +
> +       CLASS(prepare_creds, override_cred)();
> +       if (!override_cred)
> +               return ERR_PTR(-ENOMEM);
> +
> +       override_cred->fsuid = inode->i_uid;
> +       override_cred->fsgid = inode->i_gid;
> +
> +       err = security_dentry_create_files_as(dentry, mode, &dentry->d_name,
> +                                             current->cred, override_cred);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return override_creds(no_free_ptr(override_cred));
> +}
> +
> +static void ovl_revert_creds(const struct cred *old_cred)
> +{
> +       const struct cred *override_cred;
> +
> +       override_cred = revert_creds(old_cred);
> +       put_cred(override_cred);
> +}
> +

Earlier patch removed a helper by the same name that does not put_cred()
That's a backporting trap.

Maybe something like ovl_revert_create_creds()?

And ovl_prepare_create_creds()?

> +DEFINE_CLASS(prepare_creds_ovl,
> +            const struct cred *,
> +            if (!IS_ERR(_T)) ovl_revert_creds(_T),
> +            ovl_prepare_creds(dentry, inode, mode),
> +            struct dentry *dentry, struct inode *inode, umode_t mode)
> +

Maybe also matching CLASS name.

Thanks,
Amir.