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

[Christian Brauner <brauner@kernel.org>]

On Fri, Nov 14, 2025 at 01:04:22PM +0100, Amir Goldstein wrote:
> 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()?

Ok.

> 
> > +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.

Ok.