On Fri, Jul 20, 2007 at 04:22:52PM -0700, Casey Schaufler wrote:
> +int smk_access(smack_t *sub_label, smack_t *obj_label, int requested)
> +{
> +     /*
> +      * Hardcoded comparisons.
> +      *
> +      * A star subject can't access any object.
> +      * A dash subject can't access any object.
> +      */
> +     if (*sub_label == SMK_STAR)
> +             return -EACCES;

dash comment above is a holdover from earlier code? Or is code missing?

> +     /*
> +      * A star object can be accessed by any subject.
> +      */
> +     if (*obj_label == SMK_STAR)
> +             return 0;
> +     /*
> +      * An object can be accessed in any way by a subject
> +      * with the same label.
> +      */
> +     if (*sub_label == *obj_label)
> +             return 0;
> +     /*
> +      * A hat subject can read any object.
> +      * A floor object can be read by any subject.
> +      */
> +     if (*sub_label == SMK_HAT &&
> +         ((requested & (MAY_READ | MAY_EXEC)) == requested))
> +             return 0;
> +
> +     if (*obj_label == SMK_FLOOR &&
> +         ((requested & (MAY_READ | MAY_EXEC)) == requested))
> +             return 0;
> +     /*
> +      * Beyond here an explicit relationship is required.
> +      * If the requested access is contained in the available
> +      * access (e.g. read is included in readwrite) it's
> +      * good.
> +      *
> +      * Yes, that is supposed to be a single ampersand.
> +      */
> +     if ((requested & smk_get_access(sub_label, obj_label)) == requested)
> +             return 0;
> +
> +     return -EACCES;
> +}
> +
> +int smk_curacc(smack_t *obj_label, u32 mode)
> +{
> +     struct task_smack *tsp = current->security;
> +     int rc;
> +
> +     rc = smk_access(&tsp->smk_task, obj_label, mode);
> +     if (rc == 0)
> +             return 0;
> +     
> +     if (capable(CAP_MAC_OVERRIDE))
> +             return 0;
> +
> +     return rc;
> +}
> +
> +/*
> + * The value that this adds is that everything after any
> + * character that's not allowed in a smack will be null
> + */
> +smack_t smk_from_string(char *str)
> +{
> +     smack_t smack = 0LL;
> +     char *cp;
> +     int i;
> +
> +     for (cp = (char *)&smack, i = 0; i < sizeof(smack_t); str++,cp++,i++) {
> +             if (*str <= ' ' || *str > '~')
> +                     return smack;
> +             *cp = *str;
> +     }
> +     /*
> +      * Too long.
> +      */
> +     return SMK_INVALID;
> +}
> diff -uprN -X linux-2.6.22-base/Documentation/dontdiff 
> linux-2.6.22-base/security/smack/smackfs.c 
> linux-2.6.22/security/smack/smackfs.c
> --- linux-2.6.22-base/security/smack/smackfs.c        1969-12-31 
> 16:00:00.000000000 -0800
> +++ linux-2.6.22/security/smack/smackfs.c     2007-07-20 12:14:09.000000000 
> -0700
> @@ -0,0 +1,966 @@
> +/*
> + * Copyright (C) 2007 Casey Schaufler <[EMAIL PROTECTED]>
> + *
> + *   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, version 2.
> + *
> + * Author:
> + *   Casey Schaufler <[EMAIL PROTECTED]>
> + * 
> + * Special thanks to the authors of selinuxfs. They had a good idea.

Greg may be willing to extend securityfs if it doesn't yet meet your
needs.

> + *   Karl MacMillan <[EMAIL PROTECTED]>
> + *   James Morris <[EMAIL PROTECTED]>
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/vmalloc.h>
> +#include <linux/security.h>
> +#include <net/netlabel.h>
> +#include "../../net/netlabel/netlabel_domainhash.h"
> +#include <net/cipso_ipv4.h>
> +#include "smack.h"
> +
> +/*
> + * smackfs pseudo filesystem.
> + */
> +
> +
> +enum smk_inos {
> +     SMK_ROOT_INO =  2,
> +     SMK_LOAD =      3,      /* load policy */
> +     SMK_LINKS =     4,      /* symlinks */
> +     SMK_CIPSO =     5,      /* load label -> CIPSO mapping */
> +     SMK_DOI =       6,      /* CIPSO DOI */
> +     SMK_DIRECT =    7,      /* CIPSO level indicating direct label */
> +     SMK_AMBIENT =   8,      /* internet ambient label */
> +     SMK_NLTYPE =    9,      /* label scheme to use by default */
> +     SMK_TMP =       100,    /* MUST BE LAST! /smack/tmp */
> +};
> +
> +/*
> + * This is the "ambient" label for network traffic.
> + * If it isn't somehow marked, use this.
> + * It can be reset via smackfs/ambient
> + */
> +smack_t smack_net_ambient = SMK_FLOOR;
> +
> +/*
> + * This is the default packet marking scheme for network traffic.
> + * It can be reset via smackfs/nltype
> + */
> +int smack_net_nltype = NETLBL_NLTYPE_CIPSOV4;
> +
> +/*
> + * This is the level in a CIPSO header that indicates a
> + * smack label is contained directly in the category set.
> + * It can be reset via smackfs/direct
> + */
> +int smack_cipso_direct = SMACK_CIPSO_DIRECT_DEFAULT;
> +static int smk_cipso_doi_value = SMACK_CIPSO_DOI_DEFAULT;
> +
> +static struct smk_cipso_entry smack_cipso_floor = {
> +        .smk_next = NULL,
> +        .smk_smack = SMK_FLOOR,
> +        .smk_level = 0,
> +        .smk_catset = 0LL,
> +};
> +struct smk_cipso_entry *smack_cipso = &smack_cipso_floor;
> +spinlock_t smack_cipso_lock;
> +
> +struct smk_list_entry *smack_list;
> +spinlock_t smack_list_lock;
> +static int smack_list_count;
> +
> +static ssize_t smk_read_load(struct file *filp, char __user *buf,
> +                          size_t count, loff_t *ppos)
> +{
> +     ssize_t bytes;
> +     struct smk_list_entry *slp = smack_list;
> +     struct smack_rule *srp;
> +     char *result;
> +     char *cp;
> +     int realbytes = 0;
> +     
> +     bytes = (sizeof(struct smack_rule) + 4) * smack_list_count;

Any chance for wraparound here?

> +     if (bytes == 0)
> +             return 0;
> +
> +     result = kzalloc(bytes, GFP_KERNEL);
> +     if (result == NULL)
> +             return -ENOMEM;
> +
> +     for (cp = result; slp != NULL; slp = slp->smk_next) {
> +             srp = &slp->smk_rule;
> +             sprintf(cp, "%-8s %-8s",
> +                     (char *)&srp->smk_subject, (char *)&srp->smk_object);
> +             cp += strlen(cp);
> +             if (srp->smk_access != 0) 
> +                     *cp++ = ' ';
> +             if ((srp->smk_access & MAY_READ) != 0)
> +                     *cp++ = 'r';
> +             if ((srp->smk_access & MAY_WRITE) != 0)
> +                     *cp++ = 'w';
> +             if ((srp->smk_access & MAY_EXEC) != 0)
> +                     *cp++ = 'x';
> +             if ((srp->smk_access & MAY_APPEND) != 0)
> +                     *cp++ = 'a';
> +             *cp++ = '\n';
> +     }
> +     *cp++ = '\0';
> +     realbytes = strlen(result);
> +
> +     bytes = simple_read_from_buffer(buf, count, ppos, result, realbytes);
> +     
> +     kfree(result);
> +
> +     return bytes;
> +}
> +
> +/*
> + * For purposes of SMACK:
> + *    append is a form of write
> + *    exec is a form of read
> + * This isn't reflected here, but I thought I should mention it.
> + */
> +
> +static void smk_set_access(struct smack_rule *srp)
> +{
> +     struct smk_list_entry *sp;
> +     struct smk_list_entry *newp;
> +
> +     spin_lock(&smack_list_lock);
> +
> +     for (sp = smack_list; sp != NULL; sp = sp->smk_next)
> +             if (sp->smk_rule.smk_subject == srp->smk_subject &&
> +                 sp->smk_rule.smk_object == srp->smk_object) {
> +                     sp->smk_rule.smk_access = srp->smk_access;
> +                     break;
> +             }
> +
> +     if (sp == NULL) {
> +             newp = kzalloc(sizeof(struct smk_list_entry), GFP_KERNEL);

Are GFP_KERNEL allocations kosher inside a spinlock?

> +             newp->smk_rule = *srp;
> +             newp->smk_next = smack_list;
> +             smack_list = newp;
> +             smack_list_count++;
> +     }
> +
> +     spin_unlock(&smack_list_lock);
> +
> +     return;
> +}
> +
> +
> +static ssize_t smk_write_load(struct file *file, const char __user *buf,
> +                           size_t count, loff_t *ppos)
> +{
> +     struct smack_rule rule;
> +     ssize_t rc = count;
> +     char *data = NULL;
> +     char modestr[8];
> +     char *cp;
> +
> +
> +     if (!capable(CAP_MAC_OVERRIDE))
> +             return -EPERM;
> +     /*
> +      * No partial writes.
> +      */
> +     if (*ppos != 0)
> +             return -EINVAL;
> +     /*
> +      * Casey wonders about this number.
> +      */
> +     if ((count > 64 * 1024 * 1024) || (data = vmalloc(count + 1)) == NULL)
> +             return -ENOMEM;

I understand access to vmalloc memory burns TLB entries; is kmalloc()
not good enough? (Sure, 64 megs --> vmalloc, but this policy looks
really lightweight -- 64 megs feels like a lot to me :)

> +     *(data + count) = '\0';
> +
> +     if (copy_from_user(data, buf, count) == 0) {
> +             for (cp = data - 1; cp != NULL; cp = strchr(cp + 1, '\n')) {
> +                     if (*++cp == '\0')
> +                             break;

WHy the strange dance with data - 1 and ++cp?

> +                     if (sscanf(cp, "%7s %7s %7s\n",
> +                                (char *)&rule.smk_subject,
> +                                (char *)&rule.smk_object, modestr) != 3) {
> +                             printk("%s:%d bad scan\n",
> +                                     __FUNCTION__, __LINE__);
> +                             break;
> +                     }
> +                     rule.smk_subject =
> +                             smk_from_string((char *)&rule.smk_subject);
> +                     if (rule.smk_subject == SMK_INVALID)
> +                             break;
> +                     rule.smk_object =
> +                             smk_from_string((char *)&rule.smk_object);
> +                     if (rule.smk_object == SMK_INVALID)
> +                             break;
> +                     rule.smk_access = 0;
> +                     if (strchr(modestr, 'r') || strchr(modestr, 'R'))
> +                             rule.smk_access |= MAY_READ;
> +                     if (strchr(modestr, 'w') || strchr(modestr, 'W'))
> +                             rule.smk_access |= MAY_WRITE;
> +                     if (strchr(modestr, 'x') || strchr(modestr, 'X'))
> +                             rule.smk_access |= MAY_EXEC;
> +                     if (strchr(modestr, 'a') || strchr(modestr, 'A'))
> +                             rule.smk_access |= MAY_APPEND;

No invalid checks, as above?

> +                     smk_set_access(&rule);
> +                     printk("%s:%d rule %s %s 0x%x\n",

Log level?

> +                             __FUNCTION__, __LINE__,
> +                             (char *)&rule.smk_subject,
> +                             (char *)&rule.smk_object,
> +                             rule.smk_access);
> +             }
> +     }
> +     else
> +             rc = -EFAULT;
> +
> +     vfree(data);
> +     return rc;
> +}
> +
> +static struct file_operations smk_load_ops = {
> +     .read           = smk_read_load,
> +     .write          = smk_write_load,
> +};
> +
> +static char *smk_digit(char *cp)
> +{
> +     for (; *cp != '\0'; cp++)
> +             if (*cp >= '0' && *cp <= '9')
> +                     return cp;
> +
> +     return NULL;
> +}
> +
> +static int smk_cipso_doied;
> +static int smk_cipso_written;
> +
> +/*
> + * This code reaches too deeply into netlabel internals
> + * for comfort, however there is no netlabel KAPI that
> + * allows for kernel based initialization of a CIPSO DOI.
> + * Until Paul and Casey can work out an appropriate
> + * interface Smack will do it this way.
> + */
> +static void smk_cipso_doi(void)
> +{
> +     int rc;
> +     struct cipso_v4_doi *doip;
> +     struct netlbl_dom_map *ndmp;
> +     struct netlbl_audit audit_info;
> +
> +     if (smk_cipso_doied != 0)
> +             return;
> +     smk_cipso_doied = 1;
> +
> +     doip = kmalloc(sizeof(struct cipso_v4_doi), GFP_KERNEL);
> +     if (doip == NULL)
> +             panic("smack:  Failed to initialize cipso DOI.\n");
> +     doip->map.std = NULL;
> +
> +     ndmp = kmalloc(sizeof(struct netlbl_dom_map), GFP_KERNEL);
> +     if (ndmp == NULL)
> +             panic("smack:  Failed to initialize cipso ndmp.\n");
> +
> +     doip->doi = smk_cipso_doi_value;
> +     doip->type = CIPSO_V4_MAP_PASS;
> +     doip->tags[0] = CIPSO_V4_TAG_RBITMAP;
> +     for (rc = 1; rc < CIPSO_V4_TAG_MAXCNT; rc++)
> +             doip->tags[rc] = CIPSO_V4_TAG_INVALID;
> +
> +     rc = cipso_v4_doi_add(doip);
> +     if (rc != 0)
> +             printk("%s:%d add doi rc = %d\n", __FUNCTION__, __LINE__, rc);

Log levels here, too.

> +
> +     ndmp->domain = NULL;
> +     ndmp->type = NETLBL_NLTYPE_CIPSOV4;
> +     ndmp->type_def.cipsov4 = doip;
> +
> +     rc = netlbl_domhsh_remove_default(&audit_info);
> +     if (rc != 0)
> +             printk("%s:%d remove rc = %d\n", __FUNCTION__, __LINE__, rc);
> +
> +     rc = netlbl_domhsh_add_default(ndmp, &audit_info);
> +     if (rc != 0)
> +             printk("%s:%d add rc = %d\n", __FUNCTION__, __LINE__, rc);
> +}
> +
> +/*
> + * label level[/cat[,cat]]

How many cats?


Sorry the review stops here, I'll take another look later on. :)

Attachment: pgphkEtRFbYFJ.pgp
Description: PGP signature

Reply via email to