lkml.org 
[lkml]   [2009]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 39/41] union-mount: Ignore read-only file system in permission checks
    Date
    In certain cases, we check a file for write access before it has been
    copied up to the top-level fs. We don't want to fail because the
    bottom layer is read-only - of course it is - so skip that check in
    those cases.

    Thanks to Felix Fietkau <nbd@openwrt.org> for a bug fix.

    XXX - Document when to call union_permission() vs. inode_permission()
    XXX - Kinda gross. Probably a simpler solution.

    Signed-off-by: Valerie Aurora <vaurora@redhat.com>
    ---
    fs/namei.c | 21 +++++++++++++++++----
    fs/open.c | 8 ++++++--
    fs/union.c | 32 ++++++++++++++++++++++++++++++--
    include/linux/fs.h | 1 +
    include/linux/union.h | 2 ++
    5 files changed, 56 insertions(+), 8 deletions(-)

    diff --git a/fs/namei.c b/fs/namei.c
    index 61e94aa..a8d3acf 100644
    --- a/fs/namei.c
    +++ b/fs/namei.c
    @@ -230,16 +230,17 @@ int generic_permission(struct inode *inode, int mask,
    }

    /**
    - * inode_permission - check for access rights to a given inode
    + * __inode_permission - check for access rights to a given inode
    * @inode: inode to check permission on
    * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
    + * @rofs: check for read-only fs
    *
    * Used to check for read/write/execute permissions on an inode.
    * We use "fsuid" for this, letting us set arbitrary permissions
    * for filesystem access without changing the "normal" uids which
    * are used for other things.
    */
    -int inode_permission(struct inode *inode, int mask)
    +int __inode_permission(struct inode *inode, int mask, int rofs)
    {
    int retval;

    @@ -249,7 +250,7 @@ int inode_permission(struct inode *inode, int mask)
    /*
    * Nobody gets write access to a read-only fs.
    */
    - if (IS_RDONLY(inode) &&
    + if ((rofs & IS_RDONLY(inode)) &&
    (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
    return -EROFS;

    @@ -277,6 +278,18 @@ int inode_permission(struct inode *inode, int mask)
    }

    /**
    + * inode_permission - check for access rights to a given inode
    + * @inode: inode to check permission on
    + * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
    + *
    + * This version pays attention to the MS_RDONLY flag on the fs.
    + */
    +int inode_permission(struct inode *inode, int mask)
    +{
    + return __inode_permission(inode, mask, 1);
    +}
    +
    +/**
    * file_permission - check for additional access rights to a given file
    * @file: file to check access rights for
    * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
    @@ -2129,7 +2142,7 @@ int may_open(struct path *path, int acc_mode, int flag)
    break;
    }

    - error = inode_permission(inode, acc_mode);
    + error = union_permission(path, acc_mode);
    if (error)
    return error;

    diff --git a/fs/open.c b/fs/open.c
    index dd98e80..3df5a1b 100644
    --- a/fs/open.c
    +++ b/fs/open.c
    @@ -30,6 +30,7 @@
    #include <linux/audit.h>
    #include <linux/falloc.h>
    #include <linux/fs_struct.h>
    +#include <linux/union.h>

    int vfs_statfs(struct dentry *dentry, struct kstatfs *buf)
    {
    @@ -333,6 +334,7 @@ static long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
    error = security_path_truncate(&file->f_path, length,
    ATTR_MTIME|ATTR_CTIME);
    if (!error)
    + /* Already copied up for union, opened with write */
    error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
    out_putf:
    fput(file);
    @@ -493,7 +495,8 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
    goto out_path_release;
    }

    - res = inode_permission(inode, mode | MAY_ACCESS);
    + res = union_permission(&path, mode | MAY_ACCESS);
    +
    /* SuS v2 requires we report a read only fs too */
    if (res || !(mode & S_IWOTH) || special_file(inode->i_mode))
    goto out_path_release;
    @@ -507,7 +510,8 @@ SYSCALL_DEFINE3(faccessat, int, dfd, const char __user *, filename, int, mode)
    * inherently racy and know that the fs may change
    * state before we even see this result.
    */
    - if (__mnt_is_readonly(path.mnt))
    + if ((!is_unionized(path.dentry, path.mnt) &&
    + (__mnt_is_readonly(path.mnt))))
    res = -EROFS;

    out_path_release:
    diff --git a/fs/union.c b/fs/union.c
    index d56b829..8d94b22 100644
    --- a/fs/union.c
    +++ b/fs/union.c
    @@ -390,6 +390,30 @@ static int union_relookup_topmost(struct nameidata *nd, int flags)
    return err;
    }

    +
    +/**
    + * union_permission - check for access rights to a given inode
    + * @inode: inode to check permission on
    + * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC)
    + *
    + * In a union mount, the top layer is always read-write and the bottom
    + * is always read-only. Ignore the read-only flag on the lower fs.
    + *
    + * Only need for certain activities, like checking to see if write
    + * access is ok.
    + */
    +
    +int union_permission(struct path *path, int mask)
    +{
    + struct inode *inode = path->dentry->d_inode;
    +
    + if (!is_unionized(path->dentry, path->mnt))
    + return inode_permission(inode, mask);
    +
    + /* Tell __inode_permission to ignore MS_RDONLY */
    + return __inode_permission(inode, mask, 0);
    +}
    +
    /*
    * union_create_topmost - create the topmost path component
    * @nd: pointer to nameidata of the base directory
    @@ -489,6 +513,9 @@ static int union_copy_file(struct dentry *old_dentry, struct vfsmount *old_mnt,
    if (IS_ERR(new_file))
    goto fput_old;

    + /* XXX be smart by using a length param, which indicates max
    + * data we'll want (e.g., we are about to truncate to 0 or 10
    + * bytes or something */
    size = i_size_read(old_file->f_path.dentry->d_inode);
    if (((size_t)size != size) || ((ssize_t)size != size)) {
    ret = -EFBIG;
    @@ -516,7 +543,8 @@ static int union_copy_file(struct dentry *old_dentry, struct vfsmount *old_mnt,
    * The topmost directory @new_nd must already be locked. Creates the topmost
    * file if it doesn't exist yet.
    */
    -int __union_copyup(struct path *old, struct nameidata *new_nd, struct path *new)
    +int __union_copyup(struct path *old, struct nameidata *new_nd,
    + struct path *new)
    {
    struct dentry *dentry;
    int error;
    @@ -581,7 +609,7 @@ out_dput:
    * @nd: nameidata pointer to the file
    * @flags: flags given to open_namei
    */
    -int union_copyup(struct nameidata *nd, int flags)
    +int union_copyup(struct nameidata *nd, int flags /* XXX not used */)
    {
    struct qstr this;
    char *name;
    diff --git a/include/linux/fs.h b/include/linux/fs.h
    index 57690ab..38fb113 100644
    --- a/include/linux/fs.h
    +++ b/include/linux/fs.h
    @@ -2106,6 +2106,7 @@ extern void emergency_remount(void);
    extern sector_t bmap(struct inode *, sector_t);
    #endif
    extern int notify_change(struct dentry *, struct iattr *);
    +extern int __inode_permission(struct inode *inode, int mask, int rofs);
    extern int inode_permission(struct inode *, int);
    extern int generic_permission(struct inode *, int,
    int (*check_acl)(struct inode *, int));
    diff --git a/include/linux/union.h b/include/linux/union.h
    index a0656b3..92654e0 100644
    --- a/include/linux/union.h
    +++ b/include/linux/union.h
    @@ -58,6 +58,7 @@ extern struct dentry *union_create_topmost(struct nameidata *, struct qstr *,
    extern int __union_copyup(struct path *, struct nameidata *, struct path *);
    extern int union_copyup(struct nameidata *, int);
    extern int union_copyup_dir(struct path *path);
    +extern int union_permission(struct path *, int);

    #else /* CONFIG_UNION_MOUNT */

    @@ -76,6 +77,7 @@ extern int union_copyup_dir(struct path *path);
    #define __union_copyup(x, y, z) ({ BUG(); (0); })
    #define union_copyup(x, y) ({ (0); })
    #define union_copyup_dir(x) ({ BUG(); (0); })
    +#define union_permission(x, y) inode_permission((x)->dentry->d_inode, y)

    #endif /* CONFIG_UNION_MOUNT */
    #endif /* __KERNEL__ */
    --
    1.6.3.3


    \
     
     \ /
      Last update: 2009-10-21 21:47    [W:4.113 / U:0.356 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site