Have cc(1) catch attempts to modify ->f_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.

Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/file_table.c b/fs/file_table.c
index 85b53e39138debd4d495a07129e170f1224d5cd4..b223d873e48b0a0b61eefc249c5d566441adcd31 100644 (file)
--- 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 3eb0408e3400aa69028b8957fbe5067b5d36a68e..ba8bf73d2f9cfd80e518cbc354df821ffe981995 100644 (file)
--- 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 9655158c38853e3435adb77f22ce0c81c3f0aed4..f4bdf76935303635e5d7e7816f91e7f94b7806c5 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -1022,8 +1022,8 @@ cleanup_all:
        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 af514fae4e2d01b389108f3aadc178c4da61c290..1fb02c76ae09c4f3c19cdf40b39eb99f5fd1ed2e 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1082,6 +1082,8 @@ static inline int ra_has_index(struct file_ra_state *ra, pgoff_t index)
  * @f_cred: stashed credentials of creator/opener
  * @f_owner: file owner
  * @f_path: path of the file
+ * @__f_path: writable alias for @f_path; *ONLY* for core VFS and only before
+ *   the file gets open
  * @f_pos_lock: lock protecting file position
  * @f_pipe: specific to pipes
  * @f_pos: file position
@@ -1107,7 +1109,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;