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@xxxxxxxxxxx> 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@xxxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html