On Sat 06-09-25 10:14:58, Al Viro wrote:
> [last one in #work.f_path, following the merge with #work.mount and #work.path]
>
> There are very few places that have cause to do that - all in core
> VFS now, and all done to files that are not yet opened (or visible
> to anybody else, for that matter).
>
> Let's turn f_path into a union of struct path __f_path and const
> struct path f_path. It's C, not C++ - 6.5.2.3[4] in C99 and
> later explicitly allows that kind of type-punning.
>
> That way any attempts to bypass these checks will be either very
> easy to catch, or (if the bastards get sufficiently creative to
> make it hard to spot with grep alone) very clearly malicious -
> and still catchable with a bit of instrumentation for sparse.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 85b53e39138d..b223d873e48b 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -171,7 +171,7 @@ static int init_file(struct file *f, int flags, const struct cred *cred)
> * the respective member when opening the file.
> */
> mutex_init(&f->f_pos_lock);
> - memset(&f->f_path, 0, sizeof(f->f_path));
> + memset(&f->__f_path, 0, sizeof(f->f_path));
> memset(&f->f_ra, 0, sizeof(f->f_ra));
>
> f->f_flags = flags;
> @@ -319,7 +319,7 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> static void file_init_path(struct file *file, const struct path *path,
> const struct file_operations *fop)
> {
> - file->f_path = *path;
> + file->__f_path = *path;
> file->f_inode = path->dentry->d_inode;
> file->f_mapping = path->dentry->d_inode->i_mapping;
> file->f_wb_err = filemap_sample_wb_err(file->f_mapping);
> diff --git a/fs/namei.c b/fs/namei.c
> index 3eb0408e3400..ba8bf73d2f9c 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3563,8 +3563,8 @@ static struct dentry *atomic_open(struct nameidata *nd, struct dentry *dentry,
> if (nd->flags & LOOKUP_DIRECTORY)
> open_flag |= O_DIRECTORY;
>
> - file->f_path.dentry = DENTRY_NOT_SET;
> - file->f_path.mnt = nd->path.mnt;
> + file->__f_path.dentry = DENTRY_NOT_SET;
> + file->__f_path.mnt = nd->path.mnt;
> error = dir->i_op->atomic_open(dir, dentry, file,
> open_to_namei_flags(open_flag), mode);
> d_lookup_done(dentry);
> @@ -3932,8 +3932,8 @@ int vfs_tmpfile(struct mnt_idmap *idmap,
> child = d_alloc(parentpath->dentry, &slash_name);
> if (unlikely(!child))
> return -ENOMEM;
> - file->f_path.mnt = parentpath->mnt;
> - file->f_path.dentry = child;
> + file->__f_path.mnt = parentpath->mnt;
> + file->__f_path.dentry = child;
> mode = vfs_prepare_mode(idmap, dir, mode, mode, mode);
> error = dir->i_op->tmpfile(idmap, dir, file, mode);
> dput(child);
> diff --git a/fs/open.c b/fs/open.c
> index 9655158c3885..f4bdf7693530 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1022,8 +1022,8 @@ static int do_dentry_open(struct file *f,
> put_file_access(f);
> cleanup_file:
> path_put(&f->f_path);
> - f->f_path.mnt = NULL;
> - f->f_path.dentry = NULL;
> + f->__f_path.mnt = NULL;
> + f->__f_path.dentry = NULL;
> f->f_inode = NULL;
> return error;
> }
> @@ -1050,7 +1050,7 @@ int finish_open(struct file *file, struct dentry *dentry,
> {
> BUG_ON(file->f_mode & FMODE_OPENED); /* once it's opened, it's opened */
>
> - file->f_path.dentry = dentry;
> + file->__f_path.dentry = dentry;
> return do_dentry_open(file, open);
> }
> EXPORT_SYMBOL(finish_open);
> @@ -1071,7 +1071,7 @@ EXPORT_SYMBOL(finish_open);
> */
> int finish_no_open(struct file *file, struct dentry *dentry)
> {
> - file->f_path.dentry = dentry;
> + file->__f_path.dentry = dentry;
> return 0;
> }
> EXPORT_SYMBOL(finish_no_open);
> @@ -1091,7 +1091,7 @@ int vfs_open(const struct path *path, struct file *file)
> {
> int ret;
>
> - file->f_path = *path;
> + file->__f_path = *path;
> ret = do_dentry_open(file, NULL);
> if (!ret) {
> /*
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index af514fae4e2d..7fe4831b7663 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1107,7 +1107,10 @@ struct file {
> const struct cred *f_cred;
> struct fown_struct *f_owner;
> /* --- cacheline 1 boundary (64 bytes) --- */
> - struct path f_path;
> + union {
> + const struct path f_path;
> + struct path __f_path;
> + };
> union {
> /* regular files (with FMODE_ATOMIC_POS) and directories */
> struct mutex f_pos_lock;
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
