2003-05-06 16:03:17

by Stephen Smalley

[permalink] [raw]
Subject: [PATCH] Process Attribute API for Security Modules 2.5.69

This patch against 2.5.69 implements a process attribute API for
security modules via a set of nodes in a /proc/pid/attr directory.
Credit for the idea of implementing this API via /proc/pid/attr nodes
goes to Al Viro. Jan Harkes provided a nice cleanup of the
implementation to reduce the code bloat. Al, if you approve of this
change, please acknowledge. If not, please advise as to what must
change. Thanks.

fs/proc/base.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/security.h | 23 +++++
security/dummy.c | 12 +++
3 files changed, 222 insertions(+)

Index: linux-2.5/fs/proc/base.c
diff -u linux-2.5/fs/proc/base.c:1.1.1.3 linux-2.5/fs/proc/base.c:1.8
--- linux-2.5/fs/proc/base.c:1.1.1.3 Mon Apr 21 10:15:46 2003
+++ linux-2.5/fs/proc/base.c Wed Apr 30 14:26:51 2003
@@ -58,6 +58,11 @@
PROC_PID_MAPS,
PROC_PID_MOUNTS,
PROC_PID_WCHAN,
+ PROC_PID_ATTR,
+ PROC_PID_ATTR_CURRENT,
+ PROC_PID_ATTR_PREV,
+ PROC_PID_ATTR_EXEC,
+ PROC_PID_ATTR_FSCREATE,
PROC_PID_FD_DIR = 0x8000, /* 0x8000-0xffff */
};

@@ -82,11 +87,19 @@
E(PROC_PID_ROOT, "root", S_IFLNK|S_IRWXUGO),
E(PROC_PID_EXE, "exe", S_IFLNK|S_IRWXUGO),
E(PROC_PID_MOUNTS, "mounts", S_IFREG|S_IRUGO),
+ E(PROC_PID_ATTR, "attr", S_IFDIR|S_IRUGO|S_IXUGO),
#ifdef CONFIG_KALLSYMS
E(PROC_PID_WCHAN, "wchan", S_IFREG|S_IRUGO),
#endif
{0,0,NULL,0}
};
+static struct pid_entry attr_stuff[] = {
+ E(PROC_PID_ATTR_CURRENT, "current", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_PREV, "prev", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_EXEC, "exec", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_FSCREATE, "fscreate", S_IFREG|S_IRUGO|S_IWUSR),
+ {0,0,NULL,0}
+};
#undef E

static inline struct task_struct *proc_task(struct inode *inode)
@@ -961,6 +974,175 @@
.permission = proc_permission,
};

+static int proc_attr_readdir(struct file * filp,
+ void * dirent, filldir_t filldir)
+{
+ int i;
+ int pid, ino;
+ struct inode *inode = filp->f_dentry->d_inode;
+ struct pid_entry *p;
+ int ret = 0;
+
+ lock_kernel();
+
+ pid = proc_task(inode)->pid;
+ if (!pid) {
+ ret = -ENOENT;
+ goto out;
+ }
+ i = filp->f_pos;
+ switch (i) {
+ case 0:
+ if (filldir(dirent, ".", 1, i, inode->i_ino, DT_DIR) < 0)
+ goto out;
+ i++;
+ filp->f_pos++;
+ /* fall through */
+ case 1:
+ ino = fake_ino(pid, PROC_PID_INO);
+ if (filldir(dirent, "..", 2, 1, ino, DT_DIR) < 0)
+ goto out;
+ i++;
+ filp->f_pos++;
+ /* fall through */
+ default:
+ i -= 2;
+ if (i>=sizeof(attr_stuff)/sizeof(attr_stuff[0])) {
+ ret = 1;
+ goto out;
+ }
+ p = attr_stuff + i;
+ while (p->name) {
+ if (filldir(dirent, p->name, p->len, filp->f_pos,
+ fake_ino(pid, p->type), p->mode >> 12) < 0)
+ goto out;
+ filp->f_pos++;
+ p++;
+ }
+ }
+
+ ret = 1;
+out:
+ unlock_kernel();
+ return ret;
+}
+
+static ssize_t proc_pid_attr_read(struct file * file, char * buf,
+ size_t count, loff_t *ppos)
+{
+ struct inode * inode = file->f_dentry->d_inode;
+ unsigned long page;
+ ssize_t length;
+ ssize_t end;
+ struct task_struct *task = proc_task(inode);
+
+ if (count > PAGE_SIZE)
+ count = PAGE_SIZE;
+ if (!(page = __get_free_page(GFP_KERNEL)))
+ return -ENOMEM;
+
+ length = security_getprocattr(task,
+ (char*)file->f_dentry->d_name.name,
+ (void*)page, count);
+ if (length < 0) {
+ free_page(page);
+ return length;
+ }
+ /* Static 4kB (or whatever) block capacity */
+ if (*ppos >= length) {
+ free_page(page);
+ return 0;
+ }
+ if (count + *ppos > length)
+ count = length - *ppos;
+ end = count + *ppos;
+ copy_to_user(buf, (char *) page + *ppos, count);
+ *ppos = end;
+ free_page(page);
+ return count;
+}
+
+static ssize_t proc_pid_attr_write(struct file * file, const char * buf,
+ size_t count, loff_t *ppos)
+{
+ struct inode * inode = file->f_dentry->d_inode;
+ char *page;
+ ssize_t length;
+ struct task_struct *task = proc_task(inode);
+
+ if (count > PAGE_SIZE)
+ count = PAGE_SIZE;
+ if (*ppos != 0) {
+ /* No partial writes. */
+ return -EINVAL;
+ }
+ page = (char*)__get_free_page(GFP_USER);
+ if (!page)
+ return -ENOMEM;
+ length = -EFAULT;
+ if (copy_from_user(page, buf, count))
+ goto out;
+
+ length = security_setprocattr(task,
+ (char*)file->f_dentry->d_name.name,
+ (void*)page, count);
+out:
+ free_page((unsigned long) page);
+ return length;
+}
+
+static struct file_operations proc_pid_attr_operations = {
+ .read = proc_pid_attr_read,
+ .write = proc_pid_attr_write,
+};
+
+static struct dentry *proc_attr_lookup(struct inode *dir, struct dentry *dentry)
+{
+ struct inode *inode;
+ int error;
+ struct task_struct *task = proc_task(dir);
+ struct pid_entry *p;
+ struct proc_inode *ei;
+
+ error = -ENOENT;
+ inode = NULL;
+
+ for (p = attr_stuff; p->name; p++) {
+ if (p->len != dentry->d_name.len)
+ continue;
+ if (!memcmp(dentry->d_name.name, p->name, p->len))
+ break;
+ }
+ if (!p->name)
+ goto out;
+
+ error = -EINVAL;
+ inode = proc_pid_make_inode(dir->i_sb, task, p->type);
+ if (!inode)
+ goto out;
+
+ ei = PROC_I(inode);
+ inode->i_mode = p->mode;
+ inode->i_fop = &proc_pid_attr_operations;
+ dentry->d_op = &pid_dentry_operations;
+ d_add(dentry, inode);
+ if (!proc_task(dentry->d_inode)->pid)
+ d_drop(dentry);
+ return NULL;
+
+out:
+ return ERR_PTR(error);
+}
+
+static struct file_operations proc_attr_operations = {
+ .read = generic_read_dir,
+ .readdir = proc_attr_readdir,
+};
+
+static struct inode_operations proc_attr_inode_operations = {
+ .lookup = proc_attr_lookup,
+};
+
/* SMP-safe */
static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry)
{
@@ -1040,6 +1222,11 @@
break;
case PROC_PID_MOUNTS:
inode->i_fop = &proc_mounts_operations;
+ break;
+ case PROC_PID_ATTR:
+ inode->i_nlink = 2;
+ inode->i_op = &proc_attr_inode_operations;
+ inode->i_fop = &proc_attr_operations;
break;
#ifdef CONFIG_KALLSYMS
case PROC_PID_WCHAN:
Index: linux-2.5/include/linux/security.h
diff -u linux-2.5/include/linux/security.h:1.1.1.2 linux-2.5/include/linux/security.h:1.16
--- linux-2.5/include/linux/security.h:1.1.1.2 Wed Mar 19 09:54:58 2003
+++ linux-2.5/include/linux/security.h Fri Apr 18 11:17:19 2003
@@ -1123,6 +1128,9 @@

void (*d_instantiate) (struct dentry *dentry, struct inode *inode);

+ int (*getprocattr)(struct task_struct *p, char *name, void *value, size_t size);
+ int (*setprocattr)(struct task_struct *p, char *name, void *value, size_t size);
+
#ifdef CONFIG_SECURITY_NETWORK
int (*unix_stream_connect) (struct socket * sock,
struct socket * other, struct sock * newsk);
@@ -1755,6 +1769,16 @@
security_ops->d_instantiate (dentry, inode);
}

+static inline int security_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return security_ops->getprocattr(p, name, value, size);
+}
+
+static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return security_ops->setprocattr(p, name, value, size);
+}
+
static inline int security_netlink_send(struct sk_buff * skb)
{
return security_ops->netlink_send(skb);
@@ -2339,6 +2367,16 @@

static inline void security_d_instantiate (struct dentry *dentry, struct inode *inode)
{ }
+
+static inline int security_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}
+
+static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}

/*
* The netlink capability defaults need to be used inline by default
Index: linux-2.5/security/dummy.c
diff -u linux-2.5/security/dummy.c:1.1.1.2 linux-2.5/security/dummy.c:1.14
--- linux-2.5/security/dummy.c:1.1.1.2 Wed Mar 19 09:59:17 2003
+++ linux-2.5/security/dummy.c Fri Apr 18 11:17:20 2003
@@ -736,6 +741,16 @@
return;
}

+static int dummy_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}
+
+static int dummy_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}
+

struct security_operations dummy_security_ops;

@@ -860,6 +876,8 @@
set_to_dummy_if_null(ops, register_security);
set_to_dummy_if_null(ops, unregister_security);
set_to_dummy_if_null(ops, d_instantiate);
+ set_to_dummy_if_null(ops, getprocattr);
+ set_to_dummy_if_null(ops, setprocattr);
#ifdef CONFIG_SECURITY_NETWORK
set_to_dummy_if_null(ops, unix_stream_connect);
set_to_dummy_if_null(ops, unix_may_send);



--
Stephen Smalley <[email protected]>
National Security Agency


2003-05-06 20:52:14

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Process Attribute API for Security Modules 2.5.69

Stephen Smalley <[email protected]> wrote:
>
> This patch against 2.5.69 implements a process attribute API for
> security modules via a set of nodes in a /proc/pid/attr directory.

Just a few triviata:

> +static int proc_attr_readdir(struct file * filp,

Can all this be inside CONFIG_SOMETHING? It's quite a lot of code.

> + switch (i) {
> + case 0:

We often line the `case' up with the `switch' to save a tabstop.

> + if (i>=sizeof(attr_stuff)/sizeof(attr_stuff[0])) {

The ARRAY_SIZE macro does this.

> +static ssize_t proc_pid_attr_read(struct file * file, char * buf,
> + size_t count, loff_t *ppos)
> +{
> ...
> + copy_to_user(buf, (char *) page + *ppos, count);

Need to check the return value here, return a short read if something was
copied, else -EFAULT. Or just EFAULT.


2003-05-07 10:38:06

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] Process Attribute API for Security Modules 2.5.69

On Tue, May 06, 2003 at 12:13:25PM -0400, Stephen Smalley wrote:
> +static int proc_attr_readdir(struct file * filp,
> + void * dirent, filldir_t filldir)

Umm... How about having it merged with proc_base_readdir()? I.e.
have both call the common helper. Ditto for lookups.

Other than that (and missing check for copy_to_user() return value in
->read()) I don't see any problems here.

2003-05-07 14:50:51

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH] Process Attribute API for Security Modules 2.5.69

On Wed, 2003-05-07 at 06:50, [email protected]
wrote:
> Umm... How about having it merged with proc_base_readdir()? I.e.
> have both call the common helper. Ditto for lookups.
>
> Other than that (and missing check for copy_to_user() return value in
> ->read()) I don't see any problems here.

This updated patch against 2.5.69 merges the readdir and lookup routines
for proc_base and proc_attr, fixes the copy_to_user call in
proc_attr_read and proc_info_read, moves the new data and code within
CONFIG_SECURITY, and uses ARRAY_SIZE, per the comments from Al Viro and
Andrew Morton. As before, this patch implements a process attribute API
for security modules via a set of nodes in a /proc/pid/attr directory.
Credit for the idea of implementing this API via /proc/pid/attr nodes
goes to Al Viro. Jan Harkes provided a nice cleanup of the
implementation to reduce the code bloat.

fs/proc/base.c | 172 +++++++++++++++++++++++++++++++++++++++++++----
include/linux/security.h | 23 ++++++
security/dummy.c | 12 +++
3 files changed, 196 insertions(+), 11 deletions(-)

Index: linux-2.5/fs/proc/base.c
diff -u linux-2.5/fs/proc/base.c:1.1.1.3 linux-2.5/fs/proc/base.c:1.10
--- linux-2.5/fs/proc/base.c:1.1.1.3 Mon Apr 21 10:15:46 2003
+++ linux-2.5/fs/proc/base.c Wed May 7 10:53:05 2003
@@ -58,6 +58,13 @@
PROC_PID_MAPS,
PROC_PID_MOUNTS,
PROC_PID_WCHAN,
+#ifdef CONFIG_SECURITY
+ PROC_PID_ATTR,
+ PROC_PID_ATTR_CURRENT,
+ PROC_PID_ATTR_PREV,
+ PROC_PID_ATTR_EXEC,
+ PROC_PID_ATTR_FSCREATE,
+#endif
PROC_PID_FD_DIR = 0x8000, /* 0x8000-0xffff */
};

@@ -82,11 +89,23 @@
E(PROC_PID_ROOT, "root", S_IFLNK|S_IRWXUGO),
E(PROC_PID_EXE, "exe", S_IFLNK|S_IRWXUGO),
E(PROC_PID_MOUNTS, "mounts", S_IFREG|S_IRUGO),
+#ifdef CONFIG_SECURITY
+ E(PROC_PID_ATTR, "attr", S_IFDIR|S_IRUGO|S_IXUGO),
+#endif
#ifdef CONFIG_KALLSYMS
E(PROC_PID_WCHAN, "wchan", S_IFREG|S_IRUGO),
#endif
{0,0,NULL,0}
};
+#ifdef CONFIG_SECURITY
+static struct pid_entry attr_stuff[] = {
+ E(PROC_PID_ATTR_CURRENT, "current", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_PREV, "prev", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_EXEC, "exec", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_FSCREATE, "fscreate", S_IFREG|S_IRUGO|S_IWUSR),
+ {0,0,NULL,0}
+};
+#endif
#undef E

static inline struct task_struct *proc_task(struct inode *inode)
@@ -409,8 +428,10 @@
if (count + *ppos > length)
count = length - *ppos;
end = count + *ppos;
- copy_to_user(buf, (char *) page + *ppos, count);
- *ppos = end;
+ if (copy_to_user(buf, (char *) page + *ppos, count))
+ count = -EFAULT;
+ else
+ *ppos = end;
free_page(page);
return count;
}
@@ -687,14 +708,17 @@
return retval;
}

-static int proc_base_readdir(struct file * filp,
- void * dirent, filldir_t filldir)
+static int proc_pident_readdir(struct file * filp,
+ void * dirent, filldir_t filldir,
+ struct pid_entry *ents, unsigned int nents)
{
int i;
int pid;
- struct inode *inode = filp->f_dentry->d_inode;
+ struct dentry *dentry = filp->f_dentry;
+ struct inode *inode = dentry->d_inode;
struct pid_entry *p;
int ret = 0;
+ ino_t ino;

lock_kernel();

@@ -706,24 +730,26 @@
i = filp->f_pos;
switch (i) {
case 0:
- if (filldir(dirent, ".", 1, i, inode->i_ino, DT_DIR) < 0)
+ ino = inode->i_ino;
+ if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
goto out;
i++;
filp->f_pos++;
/* fall through */
case 1:
- if (filldir(dirent, "..", 2, i, PROC_ROOT_INO, DT_DIR) < 0)
+ ino = parent_ino(dentry);
+ if (filldir(dirent, "..", 2, i, ino, DT_DIR) < 0)
goto out;
i++;
filp->f_pos++;
/* fall through */
default:
i -= 2;
- if (i>=sizeof(base_stuff)/sizeof(base_stuff[0])) {
+ if (i>=nents) {
ret = 1;
goto out;
}
- p = base_stuff + i;
+ p = ents + i;
while (p->name) {
if (filldir(dirent, p->name, p->len, filp->f_pos,
fake_ino(pid, p->type), p->mode >> 12) < 0)
@@ -739,6 +765,13 @@
return ret;
}

+static int proc_base_readdir(struct file * filp,
+ void * dirent, filldir_t filldir)
+{
+ return proc_pident_readdir(filp,dirent,filldir,
+ base_stuff,ARRAY_SIZE(base_stuff));
+}
+
/* building an inode */

static int task_dumpable(struct task_struct *task)
@@ -961,8 +994,86 @@
.permission = proc_permission,
};

+#ifdef CONFIG_SECURITY
+static ssize_t proc_pid_attr_read(struct file * file, char * buf,
+ size_t count, loff_t *ppos)
+{
+ struct inode * inode = file->f_dentry->d_inode;
+ unsigned long page;
+ ssize_t length;
+ ssize_t end;
+ struct task_struct *task = proc_task(inode);
+
+ if (count > PAGE_SIZE)
+ count = PAGE_SIZE;
+ if (!(page = __get_free_page(GFP_KERNEL)))
+ return -ENOMEM;
+
+ length = security_getprocattr(task,
+ (char*)file->f_dentry->d_name.name,
+ (void*)page, count);
+ if (length < 0) {
+ free_page(page);
+ return length;
+ }
+ /* Static 4kB (or whatever) block capacity */
+ if (*ppos >= length) {
+ free_page(page);
+ return 0;
+ }
+ if (count + *ppos > length)
+ count = length - *ppos;
+ end = count + *ppos;
+ if (copy_to_user(buf, (char *) page + *ppos, count))
+ count = -EFAULT;
+ else
+ *ppos = end;
+ free_page(page);
+ return count;
+}
+
+static ssize_t proc_pid_attr_write(struct file * file, const char * buf,
+ size_t count, loff_t *ppos)
+{
+ struct inode * inode = file->f_dentry->d_inode;
+ char *page;
+ ssize_t length;
+ struct task_struct *task = proc_task(inode);
+
+ if (count > PAGE_SIZE)
+ count = PAGE_SIZE;
+ if (*ppos != 0) {
+ /* No partial writes. */
+ return -EINVAL;
+ }
+ page = (char*)__get_free_page(GFP_USER);
+ if (!page)
+ return -ENOMEM;
+ length = -EFAULT;
+ if (copy_from_user(page, buf, count))
+ goto out;
+
+ length = security_setprocattr(task,
+ (char*)file->f_dentry->d_name.name,
+ (void*)page, count);
+out:
+ free_page((unsigned long) page);
+ return length;
+}
+
+static struct file_operations proc_pid_attr_operations = {
+ .read = proc_pid_attr_read,
+ .write = proc_pid_attr_write,
+};
+
+static struct file_operations proc_attr_operations;
+static struct inode_operations proc_attr_inode_operations;
+#endif
+
/* SMP-safe */
-static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry)
+static struct dentry *proc_pident_lookup(struct inode *dir,
+ struct dentry *dentry,
+ struct pid_entry *ents)
{
struct inode *inode;
int error;
@@ -973,7 +1084,7 @@
error = -ENOENT;
inode = NULL;

- for (p = base_stuff; p->name; p++) {
+ for (p = ents; p->name; p++) {
if (p->len != dentry->d_name.len)
continue;
if (!memcmp(dentry->d_name.name, p->name, p->len))
@@ -1041,6 +1152,19 @@
case PROC_PID_MOUNTS:
inode->i_fop = &proc_mounts_operations;
break;
+#ifdef CONFIG_SECURITY
+ case PROC_PID_ATTR:
+ inode->i_nlink = 2;
+ inode->i_op = &proc_attr_inode_operations;
+ inode->i_fop = &proc_attr_operations;
+ break;
+ case PROC_PID_ATTR_CURRENT:
+ case PROC_PID_ATTR_PREV:
+ case PROC_PID_ATTR_EXEC:
+ case PROC_PID_ATTR_FSCREATE:
+ inode->i_fop = &proc_pid_attr_operations;
+ break;
+#endif
#ifdef CONFIG_KALLSYMS
case PROC_PID_WCHAN:
inode->i_fop = &proc_info_file_operations;
@@ -1062,6 +1186,10 @@
return ERR_PTR(error);
}

+static struct dentry *proc_base_lookup(struct inode *dir, struct dentry *dentry){
+ return proc_pident_lookup(dir, dentry, base_stuff);
+}
+
static struct file_operations proc_base_operations = {
.read = generic_read_dir,
.readdir = proc_base_readdir,
@@ -1070,6 +1198,28 @@
static struct inode_operations proc_base_inode_operations = {
.lookup = proc_base_lookup,
};
+
+#ifdef CONFIG_SECURITY
+static int proc_attr_readdir(struct file * filp,
+ void * dirent, filldir_t filldir)
+{
+ return proc_pident_readdir(filp,dirent,filldir,
+ attr_stuff,ARRAY_SIZE(attr_stuff));
+}
+
+static struct file_operations proc_attr_operations = {
+ .read = generic_read_dir,
+ .readdir = proc_attr_readdir,
+};
+
+static struct dentry *proc_attr_lookup(struct inode *dir, struct dentry *dentry){
+ return proc_pident_lookup(dir, dentry, attr_stuff);
+}
+
+static struct inode_operations proc_attr_inode_operations = {
+ .lookup = proc_attr_lookup,
+};
+#endif

/*
* /proc/self:
Index: linux-2.5/include/linux/security.h
diff -u linux-2.5/include/linux/security.h:1.1.1.2 linux-2.5/include/linux/security.h:1.16
--- linux-2.5/include/linux/security.h:1.1.1.2 Wed Mar 19 09:54:58 2003
+++ linux-2.5/include/linux/security.h Fri Apr 18 11:17:19 2003
@@ -1123,6 +1128,9 @@

void (*d_instantiate) (struct dentry *dentry, struct inode *inode);

+ int (*getprocattr)(struct task_struct *p, char *name, void *value, size_t size);
+ int (*setprocattr)(struct task_struct *p, char *name, void *value, size_t size);
+
#ifdef CONFIG_SECURITY_NETWORK
int (*unix_stream_connect) (struct socket * sock,
struct socket * other, struct sock * newsk);
@@ -1755,6 +1769,16 @@
security_ops->d_instantiate (dentry, inode);
}

+static inline int security_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return security_ops->getprocattr(p, name, value, size);
+}
+
+static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return security_ops->setprocattr(p, name, value, size);
+}
+
static inline int security_netlink_send(struct sk_buff * skb)
{
return security_ops->netlink_send(skb);
@@ -2339,6 +2367,16 @@

static inline void security_d_instantiate (struct dentry *dentry, struct inode *inode)
{ }
+
+static inline int security_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}
+
+static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}

/*
* The netlink capability defaults need to be used inline by default
Index: linux-2.5/security/dummy.c
diff -u linux-2.5/security/dummy.c:1.1.1.2 linux-2.5/security/dummy.c:1.14
--- linux-2.5/security/dummy.c:1.1.1.2 Wed Mar 19 09:59:17 2003
+++ linux-2.5/security/dummy.c Fri Apr 18 11:17:20 2003
@@ -736,6 +741,16 @@
return;
}

+static int dummy_getprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}
+
+static int dummy_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+ return -EINVAL;
+}
+

struct security_operations dummy_security_ops;

@@ -860,6 +876,8 @@
set_to_dummy_if_null(ops, register_security);
set_to_dummy_if_null(ops, unregister_security);
set_to_dummy_if_null(ops, d_instantiate);
+ set_to_dummy_if_null(ops, getprocattr);
+ set_to_dummy_if_null(ops, setprocattr);
#ifdef CONFIG_SECURITY_NETWORK
set_to_dummy_if_null(ops, unix_stream_connect);
set_to_dummy_if_null(ops, unix_may_send);


--
Stephen Smalley <[email protected]>
National Security Agency

2003-05-08 12:00:23

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: [PATCH] Process Attribute API for Security Modules 2.5.69

The Process Attribute API, Ext2 xattr handler, and Ext3 xattr handler look
clean, so I have no objections, either. It remains to be seen how useful
this API will be.

It will be necessary to document which security attributes are defined,
and which are their valid values. These things will eventually have to be
incorporated into e2fsck, so that after a file system check it is
guaranteed that the file system is in a consistent state.


Cheers,
Andreas.

------------------------------------------------------------------------
Andreas Gruenbacher, [email protected]
Contact information: http://www.bestbits.at/~ag/

2003-05-08 13:06:52

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] Process Attribute API for Security Modules 2.5.69

On Wed, May 07, 2003 at 11:02:46AM -0400, Stephen Smalley wrote:
> On Wed, 2003-05-07 at 06:50, [email protected]
> wrote:
> > Umm... How about having it merged with proc_base_readdir()? I.e.
> > have both call the common helper. Ditto for lookups.
> >
> > Other than that (and missing check for copy_to_user() return value in
> > ->read()) I don't see any problems here.
>
> This updated patch against 2.5.69 merges the readdir and lookup routines
> for proc_base and proc_attr, fixes the copy_to_user call in
> proc_attr_read and proc_info_read, moves the new data and code within
> CONFIG_SECURITY, and uses ARRAY_SIZE, per the comments from Al Viro and
> Andrew Morton. As before, this patch implements a process attribute API
> for security modules via a set of nodes in a /proc/pid/attr directory.
> Credit for the idea of implementing this API via /proc/pid/attr nodes
> goes to Al Viro. Jan Harkes provided a nice cleanup of the
> implementation to reduce the code bloat.

[snip]

Looks sane...

2003-05-16 19:56:05

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH] Process Attribute API for Security Modules 2.5.69

This patch, relative to the /proc/pid/attr patch against 2.5.69, fixes
the mode values of the /proc/pid/attr nodes to avoid interference by the
normal Linux access checks for these nodes (and also fixes the
/proc/pid/attr/prev mode to reflect its read-only nature). Otherwise,
when the dumpable flag is cleared by a set[ug]id or unreadable
executable, a process will lose the ability to set its own attributes
via writes to /proc/pid/attr due to a DAC failure (/proc/pid inodes are
assigned the root uid/gid if the task is not dumpable, and the original
mode only permitted the owner to write). The security module should
implement appropriate permission checking in its [gs]etprocattr hook
functions. In the case of SELinux, the setprocattr hook function only
allows a process to write to its own /proc/pid/attr nodes as well as
imposing other policy-based restrictions, and the getprocattr hook
function performs a permission check between the security labels of the
current process and target process to determine whether the operation is
permitted.

base.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.5/fs/proc/base.c
===================================================================
RCS file: /home/pal/CVS/linux-2.5/fs/proc/base.c,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- linux-2.5/fs/proc/base.c 14 May 2003 12:05:37 -0000 1.11
+++ linux-2.5/fs/proc/base.c 16 May 2003 18:34:39 -0000 1.12
@@ -99,10 +99,10 @@
};
#ifdef CONFIG_SECURITY
static struct pid_entry attr_stuff[] = {
- E(PROC_PID_ATTR_CURRENT, "current", S_IFREG|S_IRUGO|S_IWUSR),
- E(PROC_PID_ATTR_PREV, "prev", S_IFREG|S_IRUGO|S_IWUSR),
- E(PROC_PID_ATTR_EXEC, "exec", S_IFREG|S_IRUGO|S_IWUSR),
- E(PROC_PID_ATTR_FSCREATE, "fscreate", S_IFREG|S_IRUGO|S_IWUSR),
+ E(PROC_PID_ATTR_CURRENT, "current", S_IFREG|S_IRUGO|S_IWUGO),
+ E(PROC_PID_ATTR_PREV, "prev", S_IFREG|S_IRUGO),
+ E(PROC_PID_ATTR_EXEC, "exec", S_IFREG|S_IRUGO|S_IWUGO),
+ E(PROC_PID_ATTR_FSCREATE, "fscreate", S_IFREG|S_IRUGO|S_IWUGO),
{0,0,NULL,0}
};
#endif



--
Stephen Smalley <[email protected]>
National Security Agency