From 894ec8707ced240b96dc45944790fb35d9a6b03c Mon Sep 17 00:00:00 2001 From: Daniel Drake Date: Mon, 12 Dec 2005 00:37:08 -0800 Subject: [PATCH] Fix listxattr() for generic security attributes Commit f549d6c18c0e8e6cf1bf0e7a47acc1daf7e2cec1 introduced a generic fallback for security xattrs, but appears to include a subtle bug. Gentoo users with kernels with selinux compiled in, and coreutils compiled with acl support, noticed that they could not copy files on tmpfs using 'cp'. cp (compiled with acl support) copies the file, lists the extended attributes on the old file, copies them all to the new file, and then exits. However the listxattr() calls were failing with this odd behaviour: llistxattr("a.out", (nil), 0) = 17 llistxattr("a.out", 0x7fffff8c6cb0, 17) = -1 ERANGE (Numerical result out of range) I believe this is a simple problem in the logic used to check the buffer sizes; if the user sends a buffer the exact size of the data, then its ok :) This change solves the problem. More info can be found at http://bugs.gentoo.org/113138 Signed-off-by: Daniel Drake Acked-by: James Morris Acked-by: Stephen Smalley Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/xattr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/xattr.c') diff --git a/fs/xattr.c b/fs/xattr.c index a9db225..bcc2156 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -245,7 +245,7 @@ listxattr(struct dentry *d, char __user *list, size_t size) error = d->d_inode->i_op->listxattr(d, klist, size); } else { error = security_inode_listsecurity(d->d_inode, klist, size); - if (size && error >= size) + if (size && error > size) error = -ERANGE; } if (error > 0) { -- cgit v1.1 From 1b1dcc1b57a49136f118a0f16367256ff9994a69 Mon Sep 17 00:00:00 2001 From: Jes Sorensen Date: Mon, 9 Jan 2006 15:59:24 -0800 Subject: [PATCH] mutex subsystem, semaphore to mutex: VFS, ->i_sem This patch converts the inode semaphore to a mutex. I have tested it on XFS and compiled as much as one can consider on an ia64. Anyway your luck with it might be different. Modified-by: Ingo Molnar (finished the conversion) Signed-off-by: Jes Sorensen Signed-off-by: Ingo Molnar --- fs/xattr.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/xattr.c') diff --git a/fs/xattr.c b/fs/xattr.c index bcc2156..386a532 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -51,7 +51,7 @@ setxattr(struct dentry *d, char __user *name, void __user *value, } } - down(&d->d_inode->i_sem); + mutex_lock(&d->d_inode->i_mutex); error = security_inode_setxattr(d, kname, kvalue, size, flags); if (error) goto out; @@ -73,7 +73,7 @@ setxattr(struct dentry *d, char __user *name, void __user *value, fsnotify_xattr(d); } out: - up(&d->d_inode->i_sem); + mutex_unlock(&d->d_inode->i_mutex); kfree(kvalue); return error; } @@ -323,9 +323,9 @@ removexattr(struct dentry *d, char __user *name) error = security_inode_removexattr(d, kname); if (error) goto out; - down(&d->d_inode->i_sem); + mutex_lock(&d->d_inode->i_mutex); error = d->d_inode->i_op->removexattr(d, kname); - up(&d->d_inode->i_sem); + mutex_unlock(&d->d_inode->i_mutex); if (!error) fsnotify_xattr(d); } -- cgit v1.1 From 5be196e5f925dab2309530fabce69c2e562b9791 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 9 Jan 2006 20:51:55 -0800 Subject: [PATCH] add vfs_* helpers for xattr operations Add vfs_getxattr, vfs_setxattr and vfs_removexattr helpers for common checks around invocation of the xattr methods. NFSD already was missing some of the checks and there will be more soon. Signed-off-by: Christoph Hellwig Cc: James Morris (James, I haven't touched selinux yet because it's doing various odd things and I'm not sure how it would interact with the security attribute fallbacks you added. Could you investigate whether it could use vfs_getxattr or if not add a __vfs_getxattr helper to share the bits it is fine with?) For NFSv4: instead of just converting it add an nfsd_getxattr helper for the code shared by NFSv2/3 and NFSv4 ACLs. In fact that code isn't even NFS-specific, but I'll wait for more users to pop up first before moving it to common code. Signed-off-by: Christoph Hellwig Acked-by: Dave Kleikamp Signed-off-by: Adrian Bunk Signed-off-by: Neil Brown Cc: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/xattr.c | 146 +++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 93 insertions(+), 53 deletions(-) (limited to 'fs/xattr.c') diff --git a/fs/xattr.c b/fs/xattr.c index 386a532..fee804e 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -19,6 +19,96 @@ #include #include + +int +vfs_setxattr(struct dentry *dentry, char *name, void *value, + size_t size, int flags) +{ + struct inode *inode = dentry->d_inode; + int error; + + mutex_lock(&inode->i_mutex); + error = security_inode_setxattr(dentry, name, value, size, flags); + if (error) + goto out; + error = -EOPNOTSUPP; + if (inode->i_op->setxattr) { + error = inode->i_op->setxattr(dentry, name, value, size, flags); + if (!error) { + fsnotify_xattr(dentry); + security_inode_post_setxattr(dentry, name, value, + size, flags); + } + } else if (!strncmp(name, XATTR_SECURITY_PREFIX, + sizeof XATTR_SECURITY_PREFIX - 1)) { + const char *suffix = name + sizeof XATTR_SECURITY_PREFIX - 1; + error = security_inode_setsecurity(inode, suffix, value, + size, flags); + if (!error) + fsnotify_xattr(dentry); + } +out: + mutex_unlock(&inode->i_mutex); + return error; +} +EXPORT_SYMBOL_GPL(vfs_setxattr); + +ssize_t +vfs_getxattr(struct dentry *dentry, char *name, void *value, size_t size) +{ + struct inode *inode = dentry->d_inode; + int error; + + error = security_inode_getxattr(dentry, name); + if (error) + return error; + + if (inode->i_op->getxattr) + error = inode->i_op->getxattr(dentry, name, value, size); + else + error = -EOPNOTSUPP; + + if (!strncmp(name, XATTR_SECURITY_PREFIX, + sizeof XATTR_SECURITY_PREFIX - 1)) { + const char *suffix = name + sizeof XATTR_SECURITY_PREFIX - 1; + int ret = security_inode_getsecurity(inode, suffix, value, + size, error); + /* + * Only overwrite the return value if a security module + * is actually active. + */ + if (ret != -EOPNOTSUPP) + error = ret; + } + + return error; +} +EXPORT_SYMBOL_GPL(vfs_getxattr); + +int +vfs_removexattr(struct dentry *dentry, char *name) +{ + struct inode *inode = dentry->d_inode; + int error; + + if (!inode->i_op->removexattr) + return -EOPNOTSUPP; + + error = security_inode_removexattr(dentry, name); + if (error) + return error; + + mutex_lock(&inode->i_mutex); + error = inode->i_op->removexattr(dentry, name); + mutex_unlock(&inode->i_mutex); + + if (!error) + fsnotify_xattr(dentry); + return error; +} +EXPORT_SYMBOL_GPL(vfs_removexattr); + + /* * Extended attribute SET operations */ @@ -51,29 +141,7 @@ setxattr(struct dentry *d, char __user *name, void __user *value, } } - mutex_lock(&d->d_inode->i_mutex); - error = security_inode_setxattr(d, kname, kvalue, size, flags); - if (error) - goto out; - error = -EOPNOTSUPP; - if (d->d_inode->i_op && d->d_inode->i_op->setxattr) { - error = d->d_inode->i_op->setxattr(d, kname, kvalue, - size, flags); - if (!error) { - fsnotify_xattr(d); - security_inode_post_setxattr(d, kname, kvalue, - size, flags); - } - } else if (!strncmp(kname, XATTR_SECURITY_PREFIX, - sizeof XATTR_SECURITY_PREFIX - 1)) { - const char *suffix = kname + sizeof XATTR_SECURITY_PREFIX - 1; - error = security_inode_setsecurity(d->d_inode, suffix, kvalue, - size, flags); - if (!error) - fsnotify_xattr(d); - } -out: - mutex_unlock(&d->d_inode->i_mutex); + error = vfs_setxattr(d, kname, kvalue, size, flags); kfree(kvalue); return error; } @@ -147,22 +215,7 @@ getxattr(struct dentry *d, char __user *name, void __user *value, size_t size) return -ENOMEM; } - error = security_inode_getxattr(d, kname); - if (error) - goto out; - error = -EOPNOTSUPP; - if (d->d_inode->i_op && d->d_inode->i_op->getxattr) - error = d->d_inode->i_op->getxattr(d, kname, kvalue, size); - - if (!strncmp(kname, XATTR_SECURITY_PREFIX, - sizeof XATTR_SECURITY_PREFIX - 1)) { - const char *suffix = kname + sizeof XATTR_SECURITY_PREFIX - 1; - int rv = security_inode_getsecurity(d->d_inode, suffix, kvalue, - size, error); - /* Security module active: overwrite error value */ - if (rv != -EOPNOTSUPP) - error = rv; - } + error = vfs_getxattr(d, kname, kvalue, size); if (error > 0) { if (size && copy_to_user(value, kvalue, error)) error = -EFAULT; @@ -171,7 +224,6 @@ getxattr(struct dentry *d, char __user *name, void __user *value, size_t size) than XATTR_SIZE_MAX bytes. Not possible. */ error = -E2BIG; } -out: kfree(kvalue); return error; } @@ -318,19 +370,7 @@ removexattr(struct dentry *d, char __user *name) if (error < 0) return error; - error = -EOPNOTSUPP; - if (d->d_inode->i_op && d->d_inode->i_op->removexattr) { - error = security_inode_removexattr(d, kname); - if (error) - goto out; - mutex_lock(&d->d_inode->i_mutex); - error = d->d_inode->i_op->removexattr(d, kname); - mutex_unlock(&d->d_inode->i_mutex); - if (!error) - fsnotify_xattr(d); - } -out: - return error; + return vfs_removexattr(d, kname); } asmlinkage long -- cgit v1.1 From e0ad7b073eb7317e5afe0385b02dcb1d52a1eedf Mon Sep 17 00:00:00 2001 From: "akpm@osdl.org" Date: Mon, 9 Jan 2006 20:51:56 -0800 Subject: [PATCH] move xattr permission checks into the VFS ) From: Christoph Hellwig The xattr code has rather complex permission checks because the rules are very different for different attribute namespaces. This patch moves as much as we can into the generic code. Currently all the major disk based filesystems duplicate these checks, while many minor filesystems or network filesystems lack some or all of them. To do this we need defines for the extended attribute names in common code, I moved them up from JFS which had the nicest defintions. Signed-off-by: Christoph Hellwig Acked-by: Dave Kleikamp Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/xattr.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 4 deletions(-) (limited to 'fs/xattr.c') diff --git a/fs/xattr.c b/fs/xattr.c index fee804e..80eca7d 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -20,6 +20,47 @@ #include +/* + * Check permissions for extended attribute access. This is a bit complicated + * because different namespaces have very different rules. + */ +static int +xattr_permission(struct inode *inode, const char *name, int mask) +{ + /* + * We can never set or remove an extended attribute on a read-only + * filesystem or on an immutable / append-only inode. + */ + if (mask & MAY_WRITE) { + if (IS_RDONLY(inode)) + return -EROFS; + if (IS_IMMUTABLE(inode) || IS_APPEND(inode)) + return -EPERM; + } + + /* + * No restriction for security.* and system.* from the VFS. Decision + * on these is left to the underlying filesystem / security module. + */ + if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN) || + !strncmp(name, XATTR_SYSTEM_PREFIX, XATTR_SYSTEM_PREFIX_LEN)) + return 0; + + /* + * The trusted.* namespace can only accessed by a privilegued user. + */ + if (!strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN)) + return (capable(CAP_SYS_ADMIN) ? 0 : -EPERM); + + if (!strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN)) { + if (!S_ISREG(inode->i_mode) && + (!S_ISDIR(inode->i_mode) || inode->i_mode & S_ISVTX)) + return -EPERM; + } + + return permission(inode, mask, NULL); +} + int vfs_setxattr(struct dentry *dentry, char *name, void *value, size_t size, int flags) @@ -27,6 +68,10 @@ vfs_setxattr(struct dentry *dentry, char *name, void *value, struct inode *inode = dentry->d_inode; int error; + error = xattr_permission(inode, name, MAY_WRITE); + if (error) + return error; + mutex_lock(&inode->i_mutex); error = security_inode_setxattr(dentry, name, value, size, flags); if (error) @@ -40,8 +85,8 @@ vfs_setxattr(struct dentry *dentry, char *name, void *value, size, flags); } } else if (!strncmp(name, XATTR_SECURITY_PREFIX, - sizeof XATTR_SECURITY_PREFIX - 1)) { - const char *suffix = name + sizeof XATTR_SECURITY_PREFIX - 1; + XATTR_SECURITY_PREFIX_LEN)) { + const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; error = security_inode_setsecurity(inode, suffix, value, size, flags); if (!error) @@ -59,6 +104,10 @@ vfs_getxattr(struct dentry *dentry, char *name, void *value, size_t size) struct inode *inode = dentry->d_inode; int error; + error = xattr_permission(inode, name, MAY_READ); + if (error) + return error; + error = security_inode_getxattr(dentry, name); if (error) return error; @@ -69,8 +118,8 @@ vfs_getxattr(struct dentry *dentry, char *name, void *value, size_t size) error = -EOPNOTSUPP; if (!strncmp(name, XATTR_SECURITY_PREFIX, - sizeof XATTR_SECURITY_PREFIX - 1)) { - const char *suffix = name + sizeof XATTR_SECURITY_PREFIX - 1; + XATTR_SECURITY_PREFIX_LEN)) { + const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; int ret = security_inode_getsecurity(inode, suffix, value, size, error); /* @@ -94,6 +143,10 @@ vfs_removexattr(struct dentry *dentry, char *name) if (!inode->i_op->removexattr) return -EOPNOTSUPP; + error = xattr_permission(inode, name, MAY_WRITE); + if (error) + return error; + error = security_inode_removexattr(dentry, name); if (error) return error; -- cgit v1.1