lkml.org 
[lkml]   [2004]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 2/3] lsm: add bsdjail module
    * Serge E. Hallyn (serue@us.ibm.com) wrote:
    > Attached is a new version of the bsdjail patch with the requested code
    > cleanups applied.

    I noticed Andrew picked this up in -mm3, but that he had to do some diff
    cleanups (see the thread/rlim changes in his tree). If you'd like Andrew
    to pick this up, it would be courteous to get the diff clean and
    building against his tree.

    > --- linux-2.6.9-rc3-bk6/security/bsdjail.c 1969-12-31 18:00:00.000000000 -0600
    > +++ linux-2.6.9-rc3-bk6-jail/security/bsdjail.c 2004-10-07 11:30:21.000000000 -0500
    > @@ -0,0 +1,1495 @@
    > +/*
    > + * File: linux/security/bsdjail.c
    > + * Author: Serge Hallyn (serue@us.ibm.com)
    > + * Date: Sep 12, 2004
    > + *
    > + * (See Documentation/bsdjail.txt for more information)
    > + *
    > + * Copyright (C) 2004 International Business Machines <serue@us.ibm.com>
    > + *
    > + * 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/config.h>
    > +#include <linux/module.h>
    > +#include <linux/kernel.h>
    > +#include <linux/init.h>
    > +#include <linux/security.h>
    > +#include <linux/namei.h>
    > +#include <linux/namespace.h>
    > +#include <linux/proc_fs.h>
    > +#include <linux/in.h>
    > +#include <linux/in6.h>
    > +#include <linux/pagemap.h>
    > +#include <linux/ip.h>
    > +#include <net/ipv6.h>
    > +#include <linux/mount.h>
    > +#include <asm/uaccess.h>
    > +#include <linux/netdevice.h>
    > +#include <linux/inetdevice.h>
    > +#include <linux/seq_file.h>
    > +#include <linux/un.h>
    > +#include <linux/smp_lock.h>
    > +#include <linux/kref.h>

    asm/ includes after linux/

    > +
    > +static int jail_debug = 0;

    unecessary assignment to 0.

    > +MODULE_PARM(jail_debug, "i");

    use module_param

    > +MODULE_PARM_DESC(jail_debug, "Print bsd jail debugging messages.\n");
    > +
    > +#define DBG 0
    > +#define WARN 1
    > +#define bsdj_debug(how, fmt, arg... ) \
    > + do { \
    > + if ( how || jail_debug ) \
    > + printk(KERN_NOTICE "%s: %s: " fmt, \
    > + MY_NAME, __FUNCTION__, \

    Andrew has cleanup here (__FUNCTION__ ,). I just use __func__, anyway.

    > + ## arg ); \
    > + } while ( 0 )
    > +
    > +#define MY_NAME "bsdjail"
    > +
    > +/* flag to keep track of how we were registered */
    > +static int secondary = 0;

    unecessary assignment to 0

    > +/*
    > + * The task structure holding jail information.
    > + * Taskp->security points to one of these (or is null).
    > + * There is exactly one jail_struct for each jail. If >1 process
    > + * are in the same jail, they share the same jail_struct.
    > + */
    > +struct jail_struct {
    > + struct kref kref;
    > +
    > + /* these are set on writes to /proc/<pid>/attr/exec */
    > + char *root_pathname; /* char * containing path to use as jail / */
    > + char *ip4_addr_name; /* char * containing ip4 addr to use for jail */
    > + char *ip6_addr_name; /* char * containing ip6 addr to use for jail */
    > +
    > + /* these are set when a jail becomes active */
    > + __u32 addr4; /* internal form of ip4_addr_name */
    > + struct in6_addr addr6; /* internal form of ip6_addr_name */
    > +
    > + struct dentry *dentry; /* dentry of fs root */
    > + struct vfsmount *mnt; /* vfsmnt of fs root */
    > +
    > + /* Resource limits. 0 = no limit */
    > + int max_nrtask; /* maximum number of tasks within this jail. */
    > + int cur_nrtask; /* current number of tasks within this jail. */
    > + long maxtimeslice; /* max timeslice in ms for procs in this jail */
    > + long nice; /* nice level for processes in this jail */
    > + long max_data, max_memlock; /* equivalent to RLIMIT_{DATA, MEMLOCK} */
    > +/* values for the jail_flags field */
    > +#define IN_USE 1 /* if 0, task is setting up jail, not yet in it */
    > +#define GOT_IPV4 2
    > +#define GOT_IPV6 4 /* if 0, ipv4, else ipv6 */
    > + char jail_flags;
    > +};

    Could go into header. Perhaps not needed if it's all there is, and it's
    not shared anywhere though.

    > +/*
    > + * disable_jail: A jail which was in use, but has no references
    > + * left, is disabled - we free up the mountpoint and dentry, and
    > + * give up our reference on the module.
    > + *
    > + * don't need to put namespace, it will be done automatically
    > + * when the last process in jail is put.
    > + * DO need to put the dentry and vfsmount
    > + */
    > +static void
    > +disable_jail(struct jail_struct *tsec)
    > +{
    > + dput(tsec->dentry);
    > + mntput(tsec->mnt);
    > + module_put(THIS_MODULE);
    > +}
    > +
    > +
    > +static void free_jail(struct jail_struct *tsec)
    > +{
    > + if (!tsec)
    > + return;
    > +
    > + kfree(tsec->root_pathname);
    > + kfree(tsec->ip4_addr_name);
    > + kfree(tsec->ip6_addr_name);
    > + kfree(tsec);
    > +}
    > +
    > +/* release_jail:
    > + * Callback for kref_put to use for releasing a jail when its
    > + * last user exits.
    > + */
    > +static void release_jail(struct kref *kref)
    > +{
    > + struct jail_struct *tsec;
    > +
    > + tsec = container_of(kref, struct jail_struct, kref);
    > + disable_jail(tsec);
    > + free_jail(tsec);
    > +}
    > +
    > +/*
    > + * jail_task_free_security: this is the callback hooked into LSM.
    > + * If there was no task->security field for bsdjail, do nothing.
    > + * If there was, but it was never put into use, free the jail.
    > + * If there was, and the jail is in use, then decrement the usage
    > + * count, and disable and free the jail if the usage count hits 0.
    > + */
    > +static void jail_task_free_security(struct task_struct *task)
    > +{
    > + struct jail_struct *tsec;
    > +
    > + tsec = task->security;
    > +
    > + if (!tsec)
    > + return;
    > +
    > + if (!(tsec->jail_flags & IN_USE)) {
    > + /*
    > + * someone did 'echo -n x > /proc/<pid>/attr/exec' but
    > + * then forked before execing. Nuke the old info.
    > + */
    > + free_jail(tsec);
    > + task->security = NULL;
    > + return;
    > + }
    > + tsec->cur_nrtask--;
    > + /* If this was the last process in the jail, delete the jail */
    > + kref_put(&tsec->kref, release_jail);
    > +}
    > +
    > +static struct jail_struct *
    > +alloc_task_security(struct task_struct *tsk)
    > +{
    > + struct jail_struct *tsec;
    > + tsec = kmalloc(sizeof(struct jail_struct), GFP_KERNEL);
    > + if (!tsec)
    > + return ERR_PTR(-ENOMEM);

    Just return NULL, that's expected norm, plus you're not using the error
    anyway.

    > + memset(tsec, 0, sizeof(struct jail_struct));
    > + tsk->security = tsec;
    > + return tsec;
    > +}
    > +
    > +static inline int
    > +in_jail(struct task_struct *t)
    > +{
    > + struct jail_struct *tsec = t->security;
    > +
    > + if (tsec && (tsec->jail_flags & IN_USE))
    > + return 1;
    > +
    > + return 0;
    > +}
    > +
    > +/*
    > + * If a network address was passed into /proc/<pid>/attr/exec,
    > + * then process in its jail will only be allowed to bind/listen
    > + * to that address.
    > + */
    > +static void
    > +setup_netaddress(struct jail_struct *tsec)
    > +{
    > + unsigned int a, b, c, d, i;
    > + unsigned int x[8];
    > +
    > + tsec->jail_flags &= ~(GOT_IPV4 | GOT_IPV6);
    > + tsec->addr4 = 0;
    > + ipv6_addr_set(&tsec->addr6, 0, 0, 0, 0);
    > +
    > + if (tsec->ip4_addr_name) {
    > + if (sscanf(tsec->ip4_addr_name, "%u.%u.%u.%u",
    > + &a, &b, &c, &d) != 4)
    > + return;
    > + if (a>255 || b>255 || c>255 || d>255)
    > + return;
    > + tsec->addr4 = htonl((a<<24) | (b<<16) | (c<<8) | d);
    > + tsec->jail_flags |= GOT_IPV4;
    > + bsdj_debug(DBG, "Network (ipv4) set up (%s)\n",
    > + tsec->ip4_addr_name);
    > + }
    > +
    > + if (tsec->ip6_addr_name) {
    > + if (sscanf(tsec->ip6_addr_name, "%x:%x:%x:%x:%x:%x:%x:%x",
    > + &x[0], &x[1], &x[2], &x[3], &x[4], &x[5], &x[6],
    > + &x[7]) != 8) {
    > + printk(KERN_INFO "%s: bad ipv6 addr %s\n", __FUNCTION__,
    > + tsec->ip6_addr_name);
    > + return;
    > + }
    > + for (i=0; i<8; i++) {
    > + if (x[i] > 65535) {
    > + printk("%s: %x > 65535 at %d\n", __FUNCTION__, x[i], i);
    > + return;
    > + }
    > + tsec->addr6.in6_u.u6_addr16[i] = htons(x[i]);
    > + }
    > + tsec->jail_flags |= GOT_IPV6;
    > + bsdj_debug(DBG, "Network (ipv6) set up (%s)\n",
    > + tsec->ip6_addr_name);
    > + }
    > +}
    > +
    > +/*
    > + * enable_jail:
    > + * Called when a process is placed into a new jail to handle the
    > + * actual creation of the jail.
    > + * Creates namespace
    > + * Sets process root+pwd
    > + * Stores the requested ip address
    > + * Registers a unique pseudo-proc filesystem for this jail
    > + */
    > +static int enable_jail(struct task_struct *tsk)
    > +{
    > + struct nameidata nd;
    > + struct jail_struct *tsec = tsk->security;;
    ^^
    generates compile error, kill the extra semi-colon

    > + int retval = -EFAULT;
    > +
    > + if (!tsec || !tsec->root_pathname)
    > + goto out;
    > +
    > + /*
    > + * USE_JAIL_NAMESPACE: could be useful, so that future mounts outside
    > + * the jail don't affect the jail. But it's not necessary, and
    > + * requires exporting copy_namespace from fs/namespace.c
    > + *
    > + * Actually, it would also be useful for truly hiding
    > + * information about mounts which do not exist in this jail.
    > +#define USE_JAIL_NAMESPACE
    > + */
    > +#ifdef USE_JAIL_NAMESPACE
    > + bsdj_debug(DBG, "bsdjail: copying namespace.\n");
    > + retval = -EPERM;
    > + if (copy_namespace(CLONE_NEWNS, tsk))
    > + goto out;
    > + bsdj_debug(DBG, "bsdjail: copied namespace.\n");
    > +#endif
    > +
    > + /* find our new root directory */
    > + bsdj_debug(DBG, "bsdjail: looking up %s\n", tsec->root_pathname);
    > + retval = path_lookup(tsec->root_pathname, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &nd);
    > + if (retval)
    > + goto out;
    > +
    > + bsdj_debug(DBG, "bsdjail: got %s, setting root to it\n", tsec->root_pathname);
    > +
    > + /* and set the fsroot to it */
    > + set_fs_root(tsk->fs, nd.mnt, nd.dentry);
    > + set_fs_pwd(tsk->fs, nd.mnt, nd.dentry);
    > +
    > + bsdj_debug(DBG, "bsdjail: root has been set. Have fun.\n");
    > +
    > + /* set up networking */
    > + if (tsec->ip4_addr_name || tsec->ip6_addr_name)
    > + setup_netaddress(tsec);
    > +
    > + tsec->cur_nrtask = 1;
    > + if (tsec->nice)
    > + set_user_nice(current, tsec->nice);
    > + if (tsec->max_data) {
    > + current->rlim[RLIMIT_DATA].rlim_cur = tsec->max_data;
    > + current->rlim[RLIMIT_DATA].rlim_max = tsec->max_data;
    > + }
    > + if (tsec->max_memlock) {
    > + current->rlim[RLIMIT_MEMLOCK].rlim_cur = tsec->max_memlock;
    > + current->rlim[RLIMIT_MEMLOCK].rlim_max = tsec->max_memlock;
    > + }
    > + if (tsec->maxtimeslice) {
    > + current->rlim[RLIMIT_CPU].rlim_cur = tsec->maxtimeslice;
    > + current->rlim[RLIMIT_CPU].rlim_max = tsec->maxtimeslice;
    > + }
    > + /* success and end */
    > + tsec->mnt = mntget(nd.mnt);
    > + tsec->dentry = dget(nd.dentry);
    > + path_release(&nd);
    > + kref_init(&tsec->kref);
    > + tsec->jail_flags |= IN_USE;
    > +
    > + /* won't let ourselves be removed until this jail goes away */
    > + try_module_get(THIS_MODULE);
    > +
    > + return 0;
    > +
    > +out:
    > + return retval;
    > +}
    > +
    > +/*
    > + * LSM /proc/<pid>/attr hooks.
    > + * You may write into /proc/<pid>/attr/exec:
    > + * root /some/path
    > + * ip 2.2.2.2
    > + * These values will be used on the next exec() to set up your jail
    > + * (assuming you're not already in a jail)
    > + */
    > +static int
    > +jail_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
    > +{
    > + struct jail_struct *tsec = current->security;
    > + long val;
    > + int start, len;
    > +
    > + if (tsec && (tsec->jail_flags & IN_USE))
    > + return -EINVAL; /* let them guess why */
    > +
    > + if (p != current || strcmp(name, "exec"))
    > + return -EPERM;
    > +
    > + if (strncmp(value, "root ", 5) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;

    I think encoding error, testing error, then returning hardcoded error is
    wasteful. I'd change alloc_task_security api to return NULL on ENOMEM.

    > +
    > + if (tsec->root_pathname)
    > + kfree(tsec->root_pathname);
    > + start = 5;
    > + len = size-start;
    > + tsec->root_pathname = kmalloc(len+1, GFP_KERNEL);
    > + if (!tsec->root_pathname)
    > + return -ENOMEM;
    > + strlcpy(tsec->root_pathname, value+start, len+1);
    > + } else if (strncmp(value, "ip ", 3) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + if (tsec->ip4_addr_name)
    > + kfree(tsec->ip4_addr_name);
    > + start = 3;
    > + len = size-start;
    > + tsec->ip4_addr_name = kmalloc(len+1, GFP_KERNEL);
    > + if (!tsec->ip4_addr_name)
    > + return -ENOMEM;
    > + strlcpy(tsec->ip4_addr_name, value+start, len+1);
    > + } else if (strncmp(value, "ip6 ", 4) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + if (tsec->ip6_addr_name)
    > + kfree(tsec->ip6_addr_name);
    > + start = 4;
    > + len = size-start;
    > + tsec->ip6_addr_name = kmalloc(len+1, GFP_KERNEL);
    > + if (!tsec->ip6_addr_name)
    > + return -ENOMEM;
    > + strlcpy(tsec->ip6_addr_name, value+start, len+1);
    > +
    > + /* the next two are equivalent */
    > + } else if (strncmp(value, "slice ", 6) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + val = simple_strtoul(value+6, NULL, 0);
    > + tsec->maxtimeslice = val;
    > + } else if (strncmp(value, "timeslice ", 10) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + val = simple_strtoul(value+10, NULL, 0);
    > + tsec->maxtimeslice = val;
    > + } else if (strncmp(value, "nrtask ", 7) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + val = (int) simple_strtol(value+7, NULL, 0);
    > + if (val < 1)
    > + return -EINVAL;
    > + tsec->max_nrtask = val;
    > + } else if (strncmp(value, "memlock ", 8) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + val = simple_strtoul(value+8, NULL, 0);
    > + tsec->max_memlock = val;
    > + } else if (strncmp(value, "data ", 5) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + val = simple_strtoul(value+5, NULL, 0);
    > + tsec->max_data = val;
    > + } else if (strncmp(value, "nice ", 5) == 0) {
    > + if (!tsec)
    > + tsec = alloc_task_security(current);
    > + if (IS_ERR(tsec))
    > + return -ENOMEM;
    > +
    > + val = simple_strtoul(value+5, NULL, 0);
    > + tsec->nice = val;
    > + } else
    > + return -EINVAL;

    Do you need all those alloc_task_security's in there? Why not just one
    at the top? And are you convinced there's no leak on the other kmalloc
    failures?

    more after lunch.
    -chris
    --
    Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:06    [W:0.097 / U:0.032 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site