This is a request for comments on the below patch that modifies the VFS
setxattr, getxattr, and listxattr code to fall back to the security
module for security xattrs if the filesystem does not support xattrs
natively. This allows security modules to export the incore inode
security label information to userspace even if the filesystem does not
provide xattr storage, and eliminates the need to individually patch
various pseudo filesystem types to provide such access (note that this
patch removes the existing xattr code from devpts and tmpfs as it is
then no longer needed). This patch was written by a colleague who
prefers to remain anonymous.
An optional variant would be to only provide such fallbacks for getxattr
(and possibly listxattr, so that any programs that first query for the
list of attributes will see the security xattr name), but not setxattr.
That seems safer (although policy can always prevent setxattr on
particular filesystem types), but would require retaining the special
handlers in devpts and tmpfs, as they need to support setxattr, and
patching any other pseudo filesystems that need to support setxattr.
Not sure ultimately what will be the right way to handle labeling of
filesystems like sysfs, i.e. exporting setxattr and letting early
userspace label it or having the kernel internally assign labels based
on some mapping.
The patch restructures the code flow slightly to reduce duplication
between the normal path and the fallback path, but this should only have
one user-visible side effect - a program may get -EACCES rather than
-EOPNOTSUPP if policy denied access but the filesystem didn't support
the operation anyway. Note that the post_setxattr hook call is not
needed in the fallback case, as the inode_setsecurity hook call handles
the incore inode security state update directly. In contrast, we do
call fsnotify in both cases.
Let me know what you think...
---
fs/devpts/Makefile | 2 -
fs/devpts/inode.c | 17 ---------
fs/devpts/xattr_security.c | 47 ------------------------
fs/xattr.c | 83 +++++++++++++++++++++++++------------------
mm/shmem.c | 85 ---------------------------------------------
5 files changed, 50 insertions(+), 184 deletions(-)
diff -X /home/sds/dontdiff -Nrup linux-2.6.13-rc3-mm1/fs/devpts/inode.c linux-2.6.13-rc3-mm1-xattr/fs/devpts/inode.c
--- linux-2.6.13-rc3-mm1/fs/devpts/inode.c 2005-06-17 15:48:29.000000000 -0400
+++ linux-2.6.13-rc3-mm1-xattr/fs/devpts/inode.c 2005-07-15 09:37:05.000000000 -0400
@@ -18,26 +18,10 @@
#include <linux/mount.h>
#include <linux/tty.h>
#include <linux/devpts_fs.h>
-#include <linux/xattr.h>
#define DEVPTS_SUPER_MAGIC 0x1cd1
-extern struct xattr_handler devpts_xattr_security_handler;
-
-static struct xattr_handler *devpts_xattr_handlers[] = {
-#ifdef CONFIG_DEVPTS_FS_SECURITY
- &devpts_xattr_security_handler,
-#endif
- NULL
-};
-
static struct inode_operations devpts_file_inode_operations = {
-#ifdef CONFIG_DEVPTS_FS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .listxattr = generic_listxattr,
- .removexattr = generic_removexattr,
-#endif
};
static struct vfsmount *devpts_mnt;
@@ -102,7 +86,6 @@ devpts_fill_super(struct super_block *s,
s->s_blocksize_bits = 10;
s->s_magic = DEVPTS_SUPER_MAGIC;
s->s_op = &devpts_sops;
- s->s_xattr = devpts_xattr_handlers;
s->s_time_gran = 1;
inode = new_inode(s);
diff -X /home/sds/dontdiff -Nrup linux-2.6.13-rc3-mm1/fs/devpts/Makefile linux-2.6.13-rc3-mm1-xattr/fs/devpts/Makefile
--- linux-2.6.13-rc3-mm1/fs/devpts/Makefile 2005-06-17 15:48:29.000000000 -0400
+++ linux-2.6.13-rc3-mm1-xattr/fs/devpts/Makefile 2005-07-15 09:37:05.000000000 -0400
@@ -5,4 +5,4 @@
obj-$(CONFIG_UNIX98_PTYS) += devpts.o
devpts-$(CONFIG_UNIX98_PTYS) := inode.o
-devpts-$(CONFIG_DEVPTS_FS_SECURITY) += xattr_security.o
+
diff -X /home/sds/dontdiff -Nrup linux-2.6.13-rc3-mm1/fs/devpts/xattr_security.c linux-2.6.13-rc3-mm1-xattr/fs/devpts/xattr_security.c
--- linux-2.6.13-rc3-mm1/fs/devpts/xattr_security.c 2005-06-17 15:48:29.000000000 -0400
+++ linux-2.6.13-rc3-mm1-xattr/fs/devpts/xattr_security.c 1969-12-31 19:00:00.000000000 -0500
@@ -1,47 +0,0 @@
-/*
- * Security xattr support for devpts.
- *
- * Author: Stephen Smalley <sds@private>
- * Copyright (c) 2004 Red Hat, Inc., James Morris <jmorris@private>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-#include <linux/string.h>
-#include <linux/fs.h>
-#include <linux/security.h>
-#include <linux/xattr.h>
-
-static size_t
-devpts_xattr_security_list(struct inode *inode, char *list, size_t list_len,
- const char *name, size_t name_len)
-{
- return security_inode_listsecurity(inode, list, list_len);
-}
-
-static int
-devpts_xattr_security_get(struct inode *inode, const char *name,
- void *buffer, size_t size)
-{
- if (strcmp(name, "") == 0)
- return -EINVAL;
- return security_inode_getsecurity(inode, name, buffer, size);
-}
-
-static int
-devpts_xattr_security_set(struct inode *inode, const char *name,
- const void *value, size_t size, int flags)
-{
- if (strcmp(name, "") == 0)
- return -EINVAL;
- return security_inode_setsecurity(inode, name, value, size, flags);
-}
-
-struct xattr_handler devpts_xattr_security_handler = {
- .prefix = XATTR_SECURITY_PREFIX,
- .list = devpts_xattr_security_list,
- .get = devpts_xattr_security_get,
- .set = devpts_xattr_security_set,
-};
diff -X /home/sds/dontdiff -Nrup linux-2.6.13-rc3-mm1/fs/xattr.c linux-2.6.13-rc3-mm1-xattr/fs/xattr.c
--- linux-2.6.13-rc3-mm1/fs/xattr.c 2005-07-15 10:52:09.000000000 -0400
+++ linux-2.6.13-rc3-mm1-xattr/fs/xattr.c 2005-07-15 09:37:05.000000000 -0400
@@ -50,21 +50,29 @@ setxattr(struct dentry *d, char __user *
return -EFAULT;
}
}
-
+ down(&d->d_inode->i_sem);
+ 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) {
- down(&d->d_inode->i_sem);
- error = security_inode_setxattr(d, kname, kvalue, size, flags);
- if (error)
- goto out;
- error = d->d_inode->i_op->setxattr(d, kname, kvalue, size, flags);
+ 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);
+ security_inode_post_setxattr(d, kname, kvalue, size,
+ flags);
}
-out:
- up(&d->d_inode->i_sem);
+ } 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:
+ up(&d->d_inode->i_sem);
+
if (kvalue)
kfree(kvalue);
return error;
@@ -138,22 +146,25 @@ getxattr(struct dentry *d, char __user *
if (!kvalue)
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 = security_inode_getxattr(d, kname);
- if (error)
- goto out;
+ if (d->d_inode->i_op && d->d_inode->i_op->getxattr)
error = d->d_inode->i_op->getxattr(d, kname, kvalue, size);
- if (error > 0) {
- if (size && copy_to_user(value, kvalue, error))
- error = -EFAULT;
- } else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
- /* The file system tried to returned a value bigger
- than XATTR_SIZE_MAX bytes. Not possible. */
- error = -E2BIG;
- }
+ else if (!strncmp(kname, XATTR_SECURITY_PREFIX, sizeof XATTR_SECURITY_PREFIX - 1)) {
+ const char *suffix = kname + sizeof XATTR_SECURITY_PREFIX - 1;
+ error = security_inode_getsecurity(d->d_inode, suffix, kvalue, size);
}
+ if (error > 0) {
+ if (size && copy_to_user(value, kvalue, error))
+ error = -EFAULT;
+ } else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
+ /* The file system tried to returned a value bigger
+ than XATTR_SIZE_MAX bytes. Not possible. */
+ error = -E2BIG;
+ }
+
out:
if (kvalue)
kfree(kvalue);
@@ -220,22 +231,26 @@ listxattr(struct dentry *d, char __user
if (!klist)
return -ENOMEM;
}
-
+ error = security_inode_listxattr(d);
+ if (error)
+ goto out;
error = -EOPNOTSUPP;
if (d->d_inode->i_op && d->d_inode->i_op->listxattr) {
- error = security_inode_listxattr(d);
- if (error)
- goto out;
error = d->d_inode->i_op->listxattr(d, klist, size);
- if (error > 0) {
- if (size && copy_to_user(list, klist, error))
- error = -EFAULT;
- } else if (error == -ERANGE && size >= XATTR_LIST_MAX) {
- /* The file system tried to returned a list bigger
- than XATTR_LIST_MAX bytes. Not possible. */
- error = -E2BIG;
- }
+ } else {
+ error = security_inode_listsecurity(d->d_inode, klist, size);
+ if (size && error >= size)
+ error = -ERANGE;
}
+ if (error > 0) {
+ if (size && copy_to_user(list, klist, error))
+ error = -EFAULT;
+ } else if (error == -ERANGE && size >= XATTR_LIST_MAX) {
+ /* The file system tried to returned a list bigger
+ than XATTR_LIST_MAX bytes. Not possible. */
+ error = -E2BIG;
+ }
+
out:
if (klist)
kfree(klist);
diff -X /home/sds/dontdiff -Nrup linux-2.6.13-rc3-mm1/mm/shmem.c linux-2.6.13-rc3-mm1-xattr/mm/shmem.c
--- linux-2.6.13-rc3-mm1/mm/shmem.c 2005-07-15 10:52:50.000000000 -0400
+++ linux-2.6.13-rc3-mm1-xattr/mm/shmem.c 2005-07-15 09:37:05.000000000 -0400
@@ -45,7 +45,6 @@
#include <linux/swapops.h>
#include <linux/mempolicy.h>
#include <linux/namei.h>
-#include <linux/xattr.h>
#include <asm/uaccess.h>
#include <asm/div64.h>
#include <asm/pgtable.h>
@@ -179,7 +178,6 @@ static struct address_space_operations s
static struct file_operations shmem_file_operations;
static struct inode_operations shmem_inode_operations;
static struct inode_operations shmem_dir_inode_operations;
-static struct inode_operations shmem_special_inode_operations;
static struct vm_operations_struct shmem_vm_ops;
static struct backing_dev_info shmem_backing_dev_info = {
@@ -1297,7 +1295,6 @@ shmem_get_inode(struct super_block *sb,
switch (mode & S_IFMT) {
default:
- inode->i_op = &shmem_special_inode_operations;
init_special_inode(inode, mode, dev);
break;
case S_IFREG:
@@ -1824,12 +1821,6 @@ static void shmem_put_link(struct dentry
static struct inode_operations shmem_symlink_inline_operations = {
.readlink = generic_readlink,
.follow_link = shmem_follow_link_inline,
-#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .listxattr = generic_listxattr,
- .removexattr = generic_removexattr,
-#endif
};
static struct inode_operations shmem_symlink_inode_operations = {
@@ -1837,12 +1828,6 @@ static struct inode_operations shmem_sym
.readlink = generic_readlink,
.follow_link = shmem_follow_link,
.put_link = shmem_put_link,
-#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .listxattr = generic_listxattr,
- .removexattr = generic_removexattr,
-#endif
};
static int shmem_parse_options(char *options, int *mode, uid_t *uid, gid_t *gid, unsigned long *blocks, unsigned long *inodes)
@@ -1962,12 +1947,6 @@ static void shmem_put_super(struct super
sb->s_fs_info = NULL;
}
-#ifdef CONFIG_TMPFS_XATTR
-static struct xattr_handler *shmem_xattr_handlers[];
-#else
-#define shmem_xattr_handlers NULL
-#endif
-
static int shmem_fill_super(struct super_block *sb,
void *data, int silent)
{
@@ -2018,7 +1997,6 @@ static int shmem_fill_super(struct super
sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
sb->s_magic = TMPFS_MAGIC;
sb->s_op = &shmem_ops;
- sb->s_xattr = shmem_xattr_handlers;
inode = shmem_get_inode(sb, S_IFDIR | mode, 0);
if (!inode)
@@ -2107,12 +2085,6 @@ static struct file_operations shmem_file
static struct inode_operations shmem_inode_operations = {
.truncate = shmem_truncate,
.setattr = shmem_notify_change,
-#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .listxattr = generic_listxattr,
- .removexattr = generic_removexattr,
-#endif
};
static struct inode_operations shmem_dir_inode_operations = {
@@ -2126,21 +2098,6 @@ static struct inode_operations shmem_dir
.rmdir = shmem_rmdir,
.mknod = shmem_mknod,
.rename = shmem_rename,
-#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .listxattr = generic_listxattr,
- .removexattr = generic_removexattr,
-#endif
-#endif
-};
-
-static struct inode_operations shmem_special_inode_operations = {
-#ifdef CONFIG_TMPFS_XATTR
- .setxattr = generic_setxattr,
- .getxattr = generic_getxattr,
- .listxattr = generic_listxattr,
- .removexattr = generic_removexattr,
#endif
};
@@ -2166,48 +2123,6 @@ static struct vm_operations_struct shmem
};
-#ifdef CONFIG_TMPFS_SECURITY
-
-static size_t shmem_xattr_security_list(struct inode *inode, char *list, size_t list_len,
- const char *name, size_t name_len)
-{
- return security_inode_listsecurity(inode, list, list_len);
-}
-
-static int shmem_xattr_security_get(struct inode *inode, const char *name, void *buffer, size_t size)
-{
- if (strcmp(name, "") == 0)
- return -EINVAL;
- return security_inode_getsecurity(inode, name, buffer, size);
-}
-
-static int shmem_xattr_security_set(struct inode *inode, const char *name, const void *value, size_t size, int flags)
-{
- if (strcmp(name, "") == 0)
- return -EINVAL;
- return security_inode_setsecurity(inode, name, value, size, flags);
-}
-
-static struct xattr_handler shmem_xattr_security_handler = {
- .prefix = XATTR_SECURITY_PREFIX,
- .list = shmem_xattr_security_list,
- .get = shmem_xattr_security_get,
- .set = shmem_xattr_security_set,
-};
-
-#endif /* CONFIG_TMPFS_SECURITY */
-
-#ifdef CONFIG_TMPFS_XATTR
-
-static struct xattr_handler *shmem_xattr_handlers[] = {
-#ifdef CONFIG_TMPFS_SECURITY
- &shmem_xattr_security_handler,
-#endif
- NULL
-};
-
-#endif /* CONFIG_TMPFS_XATTR */
-
static struct super_block *shmem_get_sb(struct file_system_type *fs_type,
int flags, const char *dev_name, void *data)
{
--
Stephen Smalley
National Security Agency
This archive was generated by hypermail 2.1.3 : Fri Jul 15 2005 - 08:34:49 PDT