diff mbox series

ovl: implement tmpfile

Message ID 20240425101556.573616-1-mszeredi@redhat.com (mailing list archive)
State New
Headers show
Series ovl: implement tmpfile | expand

Commit Message

Miklos Szeredi April 25, 2024, 10:15 a.m. UTC
Combine inode creation with opening a file.

There are six separate objects that are being set up: the backing inode,
dentry and file and the overlay inode, dentry and file.  Cleanup in case of
an error is a bit of a challenge and is difficult to test, so careful
review is needed.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/backing-file.c            |  23 +++++++
 fs/internal.h                |   3 +
 fs/namei.c                   |   6 +-
 fs/overlayfs/dir.c           | 130 +++++++++++++++++++++++++++++++++++
 fs/overlayfs/file.c          |   3 -
 fs/overlayfs/overlayfs.h     |   3 +
 include/linux/backing-file.h |   4 ++
 7 files changed, 166 insertions(+), 6 deletions(-)

Comments

Amir Goldstein April 27, 2024, 11:58 a.m. UTC | #1
On Thu, Apr 25, 2024 at 1:16 PM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> Combine inode creation with opening a file.
>
> There are six separate objects that are being set up: the backing inode,
> dentry and file and the overlay inode, dentry and file.  Cleanup in case of
> an error is a bit of a challenge and is difficult to test, so careful
> review is needed.

Did you try running the xfstests with the t_open_tmpfiles test program?
(generic/530 generic/531)
Note that those tests also run without O_TMPFILE support, so if you run
them you should verify that they do not fall back to unlinked files.

There are also a few tests that require O_TMPFILE support:
_require_xfs_io_command "-T"
(generic/004 generic/389 generic/509)

There are some tests in src/vfs/vfstest that run sgid tests
with O_TMPFILE if it is supported.
I identified generic/696 and generic/697, but only the latter
currently runs on overlayfs.


>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/backing-file.c            |  23 +++++++
>  fs/internal.h                |   3 +
>  fs/namei.c                   |   6 +-
>  fs/overlayfs/dir.c           | 130 +++++++++++++++++++++++++++++++++++
>  fs/overlayfs/file.c          |   3 -
>  fs/overlayfs/overlayfs.h     |   3 +
>  include/linux/backing-file.h |   4 ++
>  7 files changed, 166 insertions(+), 6 deletions(-)
>
> diff --git a/fs/backing-file.c b/fs/backing-file.c
> index 740185198db3..2dc3f7477d1d 100644
> --- a/fs/backing-file.c
> +++ b/fs/backing-file.c
> @@ -52,6 +52,29 @@ struct file *backing_file_open(const struct path *user_path, int flags,
>  }
>  EXPORT_SYMBOL_GPL(backing_file_open);
>
> +struct file *backing_tmpfile_open(const struct path *user_path, int flags,
> +                                 struct mnt_idmap *real_idmap,
> +                                 const struct path *real_parentpath,
> +                                 umode_t mode, const struct cred *cred)
> +{
> +       struct file *f;
> +       int error;
> +
> +       f = alloc_empty_backing_file(flags, cred);
> +       if (IS_ERR(f))
> +               return f;
> +
> +       path_get(user_path);
> +       *backing_file_user_path(f) = *user_path;
> +       error = vfs_tmpfile(real_idmap, real_parentpath, f, mode);

We do not have a custom idmap in other backing_file helpers
don't see why we need real_idmap in this helper.
I think that should be:
mnt_idmap(real_parentpath.mnt)


> +       if (error) {
> +               fput(f);
> +               f = ERR_PTR(error);
> +       }
> +       return f;
> +}
> +EXPORT_SYMBOL(backing_tmpfile_open);
> +
>  struct backing_aio {
>         struct kiocb iocb;
>         refcount_t ref;
> diff --git a/fs/internal.h b/fs/internal.h
> index 7ca738904e34..ab2225136f60 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -62,6 +62,9 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode);
>  int do_symlinkat(struct filename *from, int newdfd, struct filename *to);
>  int do_linkat(int olddfd, struct filename *old, int newdfd,
>                         struct filename *new, int flags);
> +int vfs_tmpfile(struct mnt_idmap *idmap,
> +               const struct path *parentpath,
> +               struct file *file, umode_t mode);
>
>  /*
>   * namespace.c
> diff --git a/fs/namei.c b/fs/namei.c
> index c5b2a25be7d0..13e50b0a49d2 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3668,9 +3668,9 @@ static int do_open(struct nameidata *nd,
>   * On non-idmapped mounts or if permission checking is to be performed on the
>   * raw inode simply pass @nop_mnt_idmap.
>   */
> -static int vfs_tmpfile(struct mnt_idmap *idmap,
> -                      const struct path *parentpath,
> -                      struct file *file, umode_t mode)
> +int vfs_tmpfile(struct mnt_idmap *idmap,
> +               const struct path *parentpath,
> +               struct file *file, umode_t mode)
>  {
>         struct dentry *child;
>         struct inode *dir = d_inode(parentpath->dentry);
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 0f8b4a719237..91ac268986a9 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -14,6 +14,7 @@
>  #include <linux/posix_acl_xattr.h>
>  #include <linux/atomic.h>
>  #include <linux/ratelimit.h>
> +#include <linux/backing-file.h>
>  #include "overlayfs.h"
>
>  static unsigned short ovl_redirect_max = 256;
> @@ -1290,6 +1291,134 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
>         return err;
>  }
>
> +static int ovl_create_upper_tmpfile(struct file *file, struct dentry *dentry,
> +                                   struct inode *inode, umode_t mode)
> +{
> +       struct ovl_inode_params oip;
> +       struct path realparentpath;
> +       struct file *realfile;
> +       /* It's okay to set O_NOATIME, since the owner will be current fsuid */
> +       int flags = file->f_flags | OVL_OPEN_FLAGS;
> +
> +       ovl_path_upper(dentry->d_parent, &realparentpath);
> +
> +       if (!IS_POSIXACL(d_inode(realparentpath.dentry)))
> +               mode &= ~current_umask();
> +
> +       realfile = backing_tmpfile_open(&file->f_path, flags,
> +                                       &nop_mnt_idmap, &realparentpath, mode,
> +                                       current_cred());

Using &nop_mnt_idmap here is not only unneeded but also looks wrong.

> +       if (IS_ERR(realfile))
> +               return PTR_ERR(realfile);
> +
> +       ovl_dentry_set_upper_alias(dentry);
> +       ovl_dentry_update_reval(dentry, realfile->f_path.dentry);
> +
> +       /* ovl_get_inode() consumes the .upperdentry reference on success */
> +       oip = (struct ovl_inode_params) {
> +               .upperdentry = dget(realfile->f_path.dentry),
> +               .newinode = inode,
> +       };
> +
> +       inode = ovl_get_inode(dentry->d_sb, &oip);
> +       if (IS_ERR(inode))
> +               goto out_err;
> +
> +       /* d_tmpfile() expects inode to have a positive link count */
> +       set_nlink(inode, 1);
> +       d_tmpfile(file, inode);

Any reason not to reuse ovl_instantiate() to avoid duplicating some
of the subtlety? for example:

+       /* ovl_instantiate() consumes the .upperdentry reference on success */
+       dget(realfile->f_path.dentry)
+       err = ovl_instantiate(dentry, inode, realfile->f_path.dentry, 0, 1);
+       if (err)
+               goto out_err;

[...]

 static int ovl_instantiate(struct dentry *dentry, struct inode *inode,
-                          struct dentry *newdentry, bool hardlink)
+                          struct dentry *newdentry, bool hardlink,
bool tmpfile)
 {
        struct ovl_inode_params oip = {
                .upperdentry = newdentry,
@@ -295,6 +295,9 @@ static int ovl_instantiate(struct dentry *dentry,
struct inode *inode,
                inc_nlink(inode);
        }

+       if (tmpfile)
+               d_mark_tmpfile(dentry);
+
        d_instantiate(dentry, inode);


> +       file->private_data = realfile;
> +       return 0;
> +
> +out_err:
> +       dput(realfile->f_path.dentry);
> +       fput(realfile);
> +       return PTR_ERR(inode);
> +}
> +
> +static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
> +                             struct inode *inode, umode_t mode)
> +{
> +       int err;
> +       const struct cred *old_cred;
> +       struct cred *override_cred;
> +
> +       err = ovl_copy_up(dentry->d_parent);
> +       if (err)
> +               return err;
> +
> +       old_cred = ovl_override_creds(dentry->d_sb);
> +
> +       err = -ENOMEM;
> +       override_cred = prepare_creds();
> +       if (override_cred) {
> +               override_cred->fsuid = inode->i_uid;
> +               override_cred->fsgid = inode->i_gid;
> +               err = security_dentry_create_files_as(dentry, mode,
> +                                                     &dentry->d_name, old_cred,
> +                                                     override_cred);
> +               if (err) {
> +                       put_cred(override_cred);
> +                       goto out_revert_creds;
> +               }
> +               put_cred(override_creds(override_cred));
> +               put_cred(override_cred);
> +
> +               err = ovl_create_upper_tmpfile(file, dentry, inode, mode);
> +       }
> +out_revert_creds:
> +       revert_creds(old_cred);
> +       return err;
> +}

This also shouts unneeded and subtle code duplication to me:

 static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
-                             struct ovl_cattr *attr, bool origin)
+                             struct ovl_cattr *attr, bool origin,
+                             struct file *tmpfile)
 {
        int err;
        const struct cred *old_cred;
@@ -602,7 +606,9 @@ static int ovl_create_or_link(struct dentry
*dentry, struct inode *inode,
                put_cred(override_cred);
        }

-       if (!ovl_dentry_is_whiteout(dentry))
+       if (tmpfile)
+               err = ovl_create_upper_tmpfile(tmpfile, dentry, inode,
attr->mode);
+       else if (!ovl_dentry_is_whiteout(dentry))
                err = ovl_create_upper(dentry, inode, attr);
        else
                err = ovl_create_over_whiteout(dentry, inode, attr);

> +
> +static int ovl_dummy_open(struct inode *inode, struct file *file)
> +{
> +       return 0;
> +}
> +
> +static int ovl_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
> +                      struct file *file, umode_t mode)
> +{
> +       int err;
> +       struct dentry *dentry = file->f_path.dentry;
> +       struct inode *inode;
> +
> +       err = ovl_want_write(dentry);
> +       if (err)
> +               return err;
> +
> +       err = -ENOMEM;
> +       inode = ovl_new_inode(dentry->d_sb, mode, 0);
> +       if (!inode)
> +               goto drop_write;
> +
> +       inode_init_owner(&nop_mnt_idmap, inode, dir, mode);
> +       err = ovl_create_tmpfile(file, dentry, inode, inode->i_mode);
> +       if (err)
> +               goto put_inode;
> +
> +       /*
> +        * Check if the preallocated inode was actually used.  Having something
> +        * else assigned to the dentry shouldn't happen as that would indicate
> +        * that the backing tmpfile "leaked" out of overlayfs.
> +        */
> +       err = -EIO;
> +       if (WARN_ON(inode != d_inode(dentry)))
> +               goto put_realfile;
> +
> +       /* inode reference was transferred to dentry */
> +       inode = NULL;
> +       err = finish_open(file, dentry, ovl_dummy_open);
> +put_realfile:
> +       if (!(file->f_mode & FMODE_OPENED))
> +               fput(file->private_data);

This cleanup bit is very subtle and hard for me to review.
I wonder if there is a way to improve this subtlety?

Would it be possible to write this cleanup as:
+       if (err && file->private_data)
+               fput(file->private_data);

With a comment explaining where file->private_data is set?

Overall, I did not find any bugs, but I am hoping that the code could be
a bit easier to review and maintain.

Thanks,
Amir.
Miklos Szeredi April 29, 2024, 9:41 a.m. UTC | #2
On Sat, 27 Apr 2024 at 13:58, Amir Goldstein <amir73il@gmail.com> wrote:

> There are some tests in src/vfs/vfstest that run sgid tests
> with O_TMPFILE if it is supported.
> I identified generic/696 and generic/697, but only the latter
> currently runs on overlayfs.

Yes, I ran full xfstests, but now I also verified that they do use
O_TMPFILE on overlayfs and it works.

The only one that didn't run from these was:

generic/509       [not run] require /scratch to be valid block disk

> > +       path_get(user_path);
> > +       *backing_file_user_path(f) = *user_path;
> > +       error = vfs_tmpfile(real_idmap, real_parentpath, f, mode);
>
> We do not have a custom idmap in other backing_file helpers
> don't see why we need real_idmap in this helper.
> I think that should be:
> mnt_idmap(real_parentpath.mnt)

Yes.

> > +       realfile = backing_tmpfile_open(&file->f_path, flags,
> > +                                       &nop_mnt_idmap, &realparentpath, mode,
> > +                                       current_cred());
>
> Using &nop_mnt_idmap here is not only unneeded but also looks wrong.

Yep, no need to pass idmap down this helper, since it can be extracted
form realparentpath.

> Any reason not to reuse ovl_instantiate() to avoid duplicating some
> of the subtlety? for example:
>
> +       /* ovl_instantiate() consumes the .upperdentry reference on success */
> +       dget(realfile->f_path.dentry)
> +       err = ovl_instantiate(dentry, inode, realfile->f_path.dentry, 0, 1);
> +       if (err)
> +               goto out_err;
>
> [...]
>
>  static int ovl_instantiate(struct dentry *dentry, struct inode *inode,
> -                          struct dentry *newdentry, bool hardlink)
> +                          struct dentry *newdentry, bool hardlink,
> bool tmpfile)
>  {
>         struct ovl_inode_params oip = {
>                 .upperdentry = newdentry,
> @@ -295,6 +295,9 @@ static int ovl_instantiate(struct dentry *dentry,
> struct inode *inode,
>                 inc_nlink(inode);
>         }
>
> +       if (tmpfile)
> +               d_mark_tmpfile(dentry);
> +
>         d_instantiate(dentry, inode);
>

Okay, makes sense.


> > +static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
> > +                             struct inode *inode, umode_t mode)
> > +{
> > +       int err;
> > +       const struct cred *old_cred;
> > +       struct cred *override_cred;
> > +
> > +       err = ovl_copy_up(dentry->d_parent);
> > +       if (err)
> > +               return err;
> > +
> > +       old_cred = ovl_override_creds(dentry->d_sb);
> > +
> > +       err = -ENOMEM;
> > +       override_cred = prepare_creds();
> > +       if (override_cred) {
> > +               override_cred->fsuid = inode->i_uid;
> > +               override_cred->fsgid = inode->i_gid;
> > +               err = security_dentry_create_files_as(dentry, mode,
> > +                                                     &dentry->d_name, old_cred,
> > +                                                     override_cred);
> > +               if (err) {
> > +                       put_cred(override_cred);
> > +                       goto out_revert_creds;
> > +               }
> > +               put_cred(override_creds(override_cred));
> > +               put_cred(override_cred);
> > +
> > +               err = ovl_create_upper_tmpfile(file, dentry, inode, mode);
> > +       }
> > +out_revert_creds:
> > +       revert_creds(old_cred);
> > +       return err;
> > +}
>
> This also shouts unneeded and subtle code duplication to me:
>
>  static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
> -                             struct ovl_cattr *attr, bool origin)
> +                             struct ovl_cattr *attr, bool origin,
> +                             struct file *tmpfile)
>  {
>         int err;
>         const struct cred *old_cred;
> @@ -602,7 +606,9 @@ static int ovl_create_or_link(struct dentry
> *dentry, struct inode *inode,
>                 put_cred(override_cred);
>         }
>
> -       if (!ovl_dentry_is_whiteout(dentry))
> +       if (tmpfile)
> +               err = ovl_create_upper_tmpfile(tmpfile, dentry, inode,
> attr->mode);
> +       else if (!ovl_dentry_is_whiteout(dentry))
>                 err = ovl_create_upper(dentry, inode, attr);
>         else
>                 err = ovl_create_over_whiteout(dentry, inode, attr);


Instead I opted to extract the creds preparation into a separate helper.

> > +       /* inode reference was transferred to dentry */
> > +       inode = NULL;
> > +       err = finish_open(file, dentry, ovl_dummy_open);
> > +put_realfile:
> > +       if (!(file->f_mode & FMODE_OPENED))
> > +               fput(file->private_data);
>
> This cleanup bit is very subtle and hard for me to review.
> I wonder if there is a way to improve this subtlety?

The reason I did it this way is because fput() code checks
FMODE_OPENED and calls ->release() only if it's set.  So essentially
it's an indication whether the VFS will initiate the cleanup or we
need to do that ourselves.

> Would it be possible to write this cleanup as:
> +       if (err && file->private_data)
> +               fput(file->private_data);
>
> With a comment explaining where file->private_data is set?

If you look at do_dentry_open() there's an error condition after
setting FMODE_OPENED, that would result in the above being wrong.  In
fact that error condition cannot happen in overlayfs, due to aops
being set just for this reason.  So checking the error works in our
case, but I think that's more subtle than the FMODE_OPENED check.  A
comment would make sense, though.

>
> Overall, I did not find any bugs, but I am hoping that the code could be
> a bit easier to review and maintain.

Thanks for the review, will post a new version shortly.

Thanks,
Miklos
diff mbox series

Patch

diff --git a/fs/backing-file.c b/fs/backing-file.c
index 740185198db3..2dc3f7477d1d 100644
--- a/fs/backing-file.c
+++ b/fs/backing-file.c
@@ -52,6 +52,29 @@  struct file *backing_file_open(const struct path *user_path, int flags,
 }
 EXPORT_SYMBOL_GPL(backing_file_open);
 
+struct file *backing_tmpfile_open(const struct path *user_path, int flags,
+				  struct mnt_idmap *real_idmap,
+				  const struct path *real_parentpath,
+				  umode_t mode, const struct cred *cred)
+{
+	struct file *f;
+	int error;
+
+	f = alloc_empty_backing_file(flags, cred);
+	if (IS_ERR(f))
+		return f;
+
+	path_get(user_path);
+	*backing_file_user_path(f) = *user_path;
+	error = vfs_tmpfile(real_idmap, real_parentpath, f, mode);
+	if (error) {
+		fput(f);
+		f = ERR_PTR(error);
+	}
+	return f;
+}
+EXPORT_SYMBOL(backing_tmpfile_open);
+
 struct backing_aio {
 	struct kiocb iocb;
 	refcount_t ref;
diff --git a/fs/internal.h b/fs/internal.h
index 7ca738904e34..ab2225136f60 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -62,6 +62,9 @@  int do_mkdirat(int dfd, struct filename *name, umode_t mode);
 int do_symlinkat(struct filename *from, int newdfd, struct filename *to);
 int do_linkat(int olddfd, struct filename *old, int newdfd,
 			struct filename *new, int flags);
+int vfs_tmpfile(struct mnt_idmap *idmap,
+		const struct path *parentpath,
+		struct file *file, umode_t mode);
 
 /*
  * namespace.c
diff --git a/fs/namei.c b/fs/namei.c
index c5b2a25be7d0..13e50b0a49d2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3668,9 +3668,9 @@  static int do_open(struct nameidata *nd,
  * On non-idmapped mounts or if permission checking is to be performed on the
  * raw inode simply pass @nop_mnt_idmap.
  */
-static int vfs_tmpfile(struct mnt_idmap *idmap,
-		       const struct path *parentpath,
-		       struct file *file, umode_t mode)
+int vfs_tmpfile(struct mnt_idmap *idmap,
+		const struct path *parentpath,
+		struct file *file, umode_t mode)
 {
 	struct dentry *child;
 	struct inode *dir = d_inode(parentpath->dentry);
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 0f8b4a719237..91ac268986a9 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -14,6 +14,7 @@ 
 #include <linux/posix_acl_xattr.h>
 #include <linux/atomic.h>
 #include <linux/ratelimit.h>
+#include <linux/backing-file.h>
 #include "overlayfs.h"
 
 static unsigned short ovl_redirect_max = 256;
@@ -1290,6 +1291,134 @@  static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	return err;
 }
 
+static int ovl_create_upper_tmpfile(struct file *file, struct dentry *dentry,
+				    struct inode *inode, umode_t mode)
+{
+	struct ovl_inode_params oip;
+	struct path realparentpath;
+	struct file *realfile;
+	/* It's okay to set O_NOATIME, since the owner will be current fsuid */
+	int flags = file->f_flags | OVL_OPEN_FLAGS;
+
+	ovl_path_upper(dentry->d_parent, &realparentpath);
+
+	if (!IS_POSIXACL(d_inode(realparentpath.dentry)))
+		mode &= ~current_umask();
+
+	realfile = backing_tmpfile_open(&file->f_path, flags,
+					&nop_mnt_idmap, &realparentpath, mode,
+					current_cred());
+	if (IS_ERR(realfile))
+		return PTR_ERR(realfile);
+
+	ovl_dentry_set_upper_alias(dentry);
+	ovl_dentry_update_reval(dentry, realfile->f_path.dentry);
+
+	/* ovl_get_inode() consumes the .upperdentry reference on success */
+	oip = (struct ovl_inode_params) {
+		.upperdentry = dget(realfile->f_path.dentry),
+		.newinode = inode,
+	};
+
+	inode = ovl_get_inode(dentry->d_sb, &oip);
+	if (IS_ERR(inode))
+		goto out_err;
+
+	/* d_tmpfile() expects inode to have a positive link count */
+	set_nlink(inode, 1);
+	d_tmpfile(file, inode);
+	file->private_data = realfile;
+	return 0;
+
+out_err:
+	dput(realfile->f_path.dentry);
+	fput(realfile);
+	return PTR_ERR(inode);
+}
+
+static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
+			      struct inode *inode, umode_t mode)
+{
+	int err;
+	const struct cred *old_cred;
+	struct cred *override_cred;
+
+	err = ovl_copy_up(dentry->d_parent);
+	if (err)
+		return err;
+
+	old_cred = ovl_override_creds(dentry->d_sb);
+
+	err = -ENOMEM;
+	override_cred = prepare_creds();
+	if (override_cred) {
+		override_cred->fsuid = inode->i_uid;
+		override_cred->fsgid = inode->i_gid;
+		err = security_dentry_create_files_as(dentry, mode,
+						      &dentry->d_name, old_cred,
+						      override_cred);
+		if (err) {
+			put_cred(override_cred);
+			goto out_revert_creds;
+		}
+		put_cred(override_creds(override_cred));
+		put_cred(override_cred);
+
+		err = ovl_create_upper_tmpfile(file, dentry, inode, mode);
+	}
+out_revert_creds:
+	revert_creds(old_cred);
+	return err;
+}
+
+static int ovl_dummy_open(struct inode *inode, struct file *file)
+{
+	return 0;
+}
+
+static int ovl_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
+		       struct file *file, umode_t mode)
+{
+	int err;
+	struct dentry *dentry = file->f_path.dentry;
+	struct inode *inode;
+
+	err = ovl_want_write(dentry);
+	if (err)
+		return err;
+
+	err = -ENOMEM;
+	inode = ovl_new_inode(dentry->d_sb, mode, 0);
+	if (!inode)
+		goto drop_write;
+
+	inode_init_owner(&nop_mnt_idmap, inode, dir, mode);
+	err = ovl_create_tmpfile(file, dentry, inode, inode->i_mode);
+	if (err)
+		goto put_inode;
+
+	/*
+	 * Check if the preallocated inode was actually used.  Having something
+	 * else assigned to the dentry shouldn't happen as that would indicate
+	 * that the backing tmpfile "leaked" out of overlayfs.
+	 */
+	err = -EIO;
+	if (WARN_ON(inode != d_inode(dentry)))
+		goto put_realfile;
+
+	/* inode reference was transferred to dentry */
+	inode = NULL;
+	err = finish_open(file, dentry, ovl_dummy_open);
+put_realfile:
+	if (!(file->f_mode & FMODE_OPENED))
+		fput(file->private_data);
+put_inode:
+	iput(inode);
+drop_write:
+	ovl_drop_write(dentry);
+	return err;
+}
+
 const struct inode_operations ovl_dir_inode_operations = {
 	.lookup		= ovl_lookup,
 	.mkdir		= ovl_mkdir,
@@ -1310,4 +1439,5 @@  const struct inode_operations ovl_dir_inode_operations = {
 	.update_time	= ovl_update_time,
 	.fileattr_get	= ovl_fileattr_get,
 	.fileattr_set	= ovl_fileattr_set,
+	.tmpfile	= ovl_tmpfile,
 };
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 05536964d37f..1a411cae57ed 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -24,9 +24,6 @@  static char ovl_whatisit(struct inode *inode, struct inode *realinode)
 		return 'm';
 }
 
-/* No atime modification on underlying */
-#define OVL_OPEN_FLAGS (O_NOATIME)
-
 static struct file *ovl_open_realfile(const struct file *file,
 				      const struct path *realpath)
 {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index ee949f3e7c77..0bfe35da4b7b 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -175,6 +175,9 @@  static inline int ovl_metadata_digest_size(const struct ovl_metacopy *metacopy)
 	return (int)metacopy->len - OVL_METACOPY_MIN_SIZE;
 }
 
+/* No atime modification on underlying */
+#define OVL_OPEN_FLAGS (O_NOATIME)
+
 extern const char *const ovl_xattr_table[][2];
 static inline const char *ovl_xattr(struct ovl_fs *ofs, enum ovl_xattr ox)
 {
diff --git a/include/linux/backing-file.h b/include/linux/backing-file.h
index 3f1fe1774f1b..0f59f11a5a3f 100644
--- a/include/linux/backing-file.h
+++ b/include/linux/backing-file.h
@@ -22,6 +22,10 @@  struct backing_file_ctx {
 struct file *backing_file_open(const struct path *user_path, int flags,
 			       const struct path *real_path,
 			       const struct cred *cred);
+struct file *backing_tmpfile_open(const struct path *user_path, int flags,
+				  struct mnt_idmap *real_idmap,
+				  const struct path *real_parentpath,
+				  umode_t mode, const struct cred *cred);
 ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter,
 			       struct kiocb *iocb, int flags,
 			       struct backing_file_ctx *ctx);