| Message ID | 20220920193632.2215598-4-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse tmpfile | expand |
On Tue, Sep 20, 2022 at 09:36:26PM +0200, Miklos Szeredi wrote: > This helper unifies tmpfile creation with opening. > > Existing vfs_tmpfile() callers outside of fs/namei.c will be converted to > using this helper. There are two such callers: cachefile and overlayfs. > > The cachefiles code currently uses the open_with_fake_path() helper to open > the tmpfile, presumably to disable accounting of the open file. Overlayfs > uses tmpfile for copy_up, which means these struct file instances will be > short lived, hence it doesn't really matter if they are accounted or not. > Disable accounting in this helper to, which should be okay for both caller. > > Add MAY_OPEN permission checking for consistency. Like for create(2) > read/write permissions are not checked. > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > --- > fs/namei.c | 41 +++++++++++++++++++++++++++++++++++++++++ > include/linux/fs.h | 4 ++++ > 2 files changed, 45 insertions(+) > > diff --git a/fs/namei.c b/fs/namei.c > index 53b4bc094db2..5e4a0c59eef6 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -3624,6 +3624,47 @@ struct dentry *vfs_tmpfile(struct user_namespace *mnt_userns, > } > EXPORT_SYMBOL(vfs_tmpfile); > > +/** > + * tmpfile_open - open a tmpfile for kernel internal use > + * @mnt_userns: user namespace of the mount the inode was found from > + * @parentpath: path of the base directory > + * @mode: mode of the new tmpfile > + * @open_flag: flags > + * @cred: credentials for open > + * > + * Create and open a temporary file. The file is not accounted in nr_files, > + * hence this is only for kernel internal use, and must not be installed into > + * file tables or such. > + */ > +struct file *tmpfile_open(struct user_namespace *mnt_userns, > + const struct path *parentpath, > + umode_t mode, int open_flag, const struct cred *cred) > +{ > + struct file *file; > + int error; > + struct path path = { .mnt = parentpath->mnt }; > + > + path.dentry = vfs_tmpfile(mnt_userns, parentpath->dentry, mode, open_flag); > + if (IS_ERR(path.dentry)) > + return ERR_CAST(path.dentry); > + > + error = may_open(mnt_userns, &path, 0, open_flag); > + file = ERR_PTR(error); > + if (error) > + goto out_dput; > + > + /* > + * This relies on the "noaccount" property of fake open, otherwise > + * equivalent to dentry_open(). > + */ > + file = open_with_fake_path(&path, open_flag, d_inode(path.dentry), cred); > +out_dput: > + dput(path.dentry); > + > + return file; > +} > +EXPORT_SYMBOL(tmpfile_open); Feels like this could be simplified while being equally legible to something like: /** * tmpfile_open - open a tmpfile for kernel internal use * @mnt_userns: user namespace of the mount the inode was found from * @parentpath: path of the base directory * @mode: mode of the new tmpfile * @open_flag: flags * @cred: credentials for open * * Create and open a temporary file. The file is not accounted in nr_files, * hence this is only for kernel internal use, and must not be installed into * file tables or such. * * The helper relies on the "noaccount" property of open_with_fake_path(). * Otherwise it is equivalent to dentry_open(). * * Return: Opened tmpfile on success, error pointer on failure. */ struct file *tmpfile_open(struct user_namespace *mnt_userns, const struct path *parentpath, umode_t mode, int open_flag, const struct cred *cred) { struct file *file; int error; struct path path = { .mnt = parentpath->mnt }; path.dentry = vfs_tmpfile(mnt_userns, parentpath->dentry, mode, open_flag); if (IS_ERR(path.dentry)) return ERR_CAST(path.dentry); error = may_open(mnt_userns, &path, 0, open_flag); if (!error) file = open_with_fake_path(&path, open_flag, d_inode(path.dentry), cred); else file = ERR_PTR(error); dput(path.dentry); return file; } EXPORT_SYMBOL(tmpfile_open);
On Wed, 21 Sept 2022 at 10:09, Christian Brauner <brauner@kernel.org> wrote: > > Feels like this could be simplified while being equally legible to > something like: > > /** > * tmpfile_open - open a tmpfile for kernel internal use > * @mnt_userns: user namespace of the mount the inode was found from > * @parentpath: path of the base directory > * @mode: mode of the new tmpfile > * @open_flag: flags > * @cred: credentials for open > * > * Create and open a temporary file. The file is not accounted in nr_files, > * hence this is only for kernel internal use, and must not be installed into > * file tables or such. > * > * The helper relies on the "noaccount" property of open_with_fake_path(). > * Otherwise it is equivalent to dentry_open(). > * > * Return: Opened tmpfile on success, error pointer on failure. > */ > struct file *tmpfile_open(struct user_namespace *mnt_userns, > const struct path *parentpath, > umode_t mode, int open_flag, const struct cred *cred) > { > struct file *file; > int error; > struct path path = { .mnt = parentpath->mnt }; > > path.dentry = vfs_tmpfile(mnt_userns, parentpath->dentry, mode, open_flag); > if (IS_ERR(path.dentry)) > return ERR_CAST(path.dentry); > > error = may_open(mnt_userns, &path, 0, open_flag); > if (!error) > file = open_with_fake_path(&path, open_flag, d_inode(path.dentry), cred); > else > file = ERR_PTR(error); > dput(path.dentry); > return file; > } > EXPORT_SYMBOL(tmpfile_open); Okay, but this code gets completely rewritten in 7/9, so maybe not worth bothering about. Thanks, Miklos
diff --git a/fs/namei.c b/fs/namei.c index 53b4bc094db2..5e4a0c59eef6 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3624,6 +3624,47 @@ struct dentry *vfs_tmpfile(struct user_namespace *mnt_userns, } EXPORT_SYMBOL(vfs_tmpfile); +/** + * tmpfile_open - open a tmpfile for kernel internal use + * @mnt_userns: user namespace of the mount the inode was found from + * @parentpath: path of the base directory + * @mode: mode of the new tmpfile + * @open_flag: flags + * @cred: credentials for open + * + * Create and open a temporary file. The file is not accounted in nr_files, + * hence this is only for kernel internal use, and must not be installed into + * file tables or such. + */ +struct file *tmpfile_open(struct user_namespace *mnt_userns, + const struct path *parentpath, + umode_t mode, int open_flag, const struct cred *cred) +{ + struct file *file; + int error; + struct path path = { .mnt = parentpath->mnt }; + + path.dentry = vfs_tmpfile(mnt_userns, parentpath->dentry, mode, open_flag); + if (IS_ERR(path.dentry)) + return ERR_CAST(path.dentry); + + error = may_open(mnt_userns, &path, 0, open_flag); + file = ERR_PTR(error); + if (error) + goto out_dput; + + /* + * This relies on the "noaccount" property of fake open, otherwise + * equivalent to dentry_open(). + */ + file = open_with_fake_path(&path, open_flag, d_inode(path.dentry), cred); +out_dput: + dput(path.dentry); + + return file; +} +EXPORT_SYMBOL(tmpfile_open); + static int do_tmpfile(struct nameidata *nd, unsigned flags, const struct open_flags *op, struct file *file) diff --git a/include/linux/fs.h b/include/linux/fs.h index 9eced4cc286e..34e7a189565b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2007,6 +2007,10 @@ static inline int vfs_whiteout(struct user_namespace *mnt_userns, struct dentry *vfs_tmpfile(struct user_namespace *mnt_userns, struct dentry *dentry, umode_t mode, int open_flag); +struct file *tmpfile_open(struct user_namespace *mnt_userns, + const struct path *parentpath, + umode_t mode, int open_flag, const struct cred *cred); + int vfs_mkobj(struct dentry *, umode_t, int (*f)(struct dentry *, umode_t, void *), void *);
This helper unifies tmpfile creation with opening. Existing vfs_tmpfile() callers outside of fs/namei.c will be converted to using this helper. There are two such callers: cachefile and overlayfs. The cachefiles code currently uses the open_with_fake_path() helper to open the tmpfile, presumably to disable accounting of the open file. Overlayfs uses tmpfile for copy_up, which means these struct file instances will be short lived, hence it doesn't really matter if they are accounted or not. Disable accounting in this helper to, which should be okay for both caller. Add MAY_OPEN permission checking for consistency. Like for create(2) read/write permissions are not checked. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/namei.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/fs.h | 4 ++++ 2 files changed, 45 insertions(+)