On Sun, Jul 22, 2007 at 09:44:49PM -0700, Casey Schaufler wrote: > > I appears that everyone else took the weekend to read > "Deathly Hallows"* as it's been pretty quiet here. Well, > my wife took first dibs on our copy so I did some polishing > on smack instead. Since no one complained about the size last
Sorry to hear she got to it first :) I'll try to not spoil it..
> +/*
> + * mapping for symlinks
> +#define SMK_TMPPATH_SIZE 32
> +#define SMK_TMPPATH_ROOT "/moldy/"
This define isn't used here, is it used in the userspace?
> + /*
> + * There will be only one smackfs. Casey says so.
> + */
> + smk_sb = sb;
:)
> + /*
> + * Create a directory for /smack/tmp
> + */
> + smk_add_symlink("tmp", "/moldy/");
I'm confused :)
> +/*
> + * Inode smack data
> + */
> +struct inode_smack {
> + smack_t smk_inode; /* label of the fso */
> + spinlock_t smk_lock; /* initialization lock */
> + int smk_flags; /* smack inode flags */
> +};
No chance to use the inode's locks?
> +/*
> + * There are not enough CAP bits available to make this
> + * real, so Casey borrowed the capability that looks to
> + * him like it has the best balance of similarity amd
> + * low use.
> + */
> +#define CAP_MAC_OVERRIDE CAP_LINUX_IMMUTABLE
Perhaps someday we should explore 64 bit cap space..
> +static smack_t *free_smack_t(smack_t *sp)
> +{
> + if (sp != NULL)
> + kfree(sp);
kfree already does this checking :)
> + return NULL;
> +}
> +
> +static void smack_sb_free_security(struct super_block *sb)
> +{
> + if (sb->s_security == NULL)
> + return;
> +
> + kfree(sb->s_security);
> + sb->s_security = NULL;
> +}
kfree doesn't need this, but it does avoid setting a NULL back to NULL.
> +static int smack_sb_kern_mount(struct super_block *sb, void *data)
> +{
> + int rc;
> + struct dentry *root = sb->s_root;
> + struct inode *inode = root->d_inode;
> + struct superblock_smack *sp = sb->s_security;
> + struct inode_smack *isp;
> + smack_t smack = SMK_UNSET;
> + char *op;
> + char *commap;
> +
> + if (sp == NULL) {
> + rc = smack_sb_alloc_security(sb);
> + if (rc != 0)
> + return rc;
> + }
> + if (sp->smk_initialized != 0)
> + return 0;
> + if (inode == NULL)
> + return 0;
> +
> + sp->smk_initialized = 1;
> +
> + /*
> + * Not everyone supports extended attributes.
> + */
> + if (inode->i_op->getxattr != NULL) {
> + rc = smk_fetch(inode, root, &smack);
> + switch (rc) {
> + case 0:
> + case -ENODATA:
> + case -EOPNOTSUPP:
> + break;
> + case sizeof(smack_t):
What does this case do?
> + default:
> + printk(KERN_WARNING "%s:%d (dev %s, type "
> + "%s) \"%s\" rc = %d\n", __FUNCTION__, __LINE__,
> + sb->s_id, sb->s_type->name, (char *)&smack, rc);
> + break;
> + }
> + }
> + switch (sbp->s_magic) {
> + case SMACK_MAGIC:
> + /*
> + * Casey says that it's a little embarassing
> + * that the smack file system doesn't do
> + * extended attributes.
> + */
:)
> + case DEVPTS_SUPER_MAGIC:
> + /*
> + * Casey says this is here because
> + * devpts is "clean" of security hooks.
> + *
> + * pty's need to be handled for real.
> + * Using "*" is expedient for bring-up.
> + *
> + */
> + final = *csp;
> + break;
Old comment?
> +static void smack_sk_free_security(struct sock *sk)
> +{
> + if (sk->sk_security == NULL)
> + return;
> +
> + kfree(sk->sk_security);
> + sk->sk_security = NULL;
> +}
kfree's fine with NULL
> +static void smack_to_secattr(smack_t smack, struct netlbl_lsm_secattr *nlsp)
> +{
> + struct smk_cipso_entry *scp;
> +
> + switch (smack_net_nltype) {
> + case NETLBL_NLTYPE_CIPSOV4:
> + nlsp->domain = NULL;
> + nlsp->flags = NETLBL_SECATTR_DOMAIN;
> + nlsp->flags |= NETLBL_SECATTR_MLS_LVL;
> +
> + for (scp = smack_cipso; scp != NULL; scp = scp->smk_next)
> + if (scp->smk_smack == smack)
> + break;
> +
> + if (scp != NULL) {
> + nlsp->mls_lvl = scp->smk_level;
> + smack_set_catset(scp->smk_catset, nlsp);
> + }
> + else {
} else { please
> + nlsp->mls_lvl = smack_cipso_direct;
> + smack_set_catset(smack, nlsp);
> + }
> + break;
> + case NETLBL_NLTYPE_NONE:
> + case NETLBL_NLTYPE_UNLABELED:
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
> +{
> + smack_t *ssp = smack_of_shm(shp);
> + int rc;
> +
> + if (ssp == NULL)
> + return 0;
> +
> + rc = smk_curacc(ssp, MAY_READWRITE);
> + return rc;
> +}
No read-only or write-only shm mappings?
> +static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
> +{
> + smack_t *ssp = smack_of_shm(shp);
> + int rc;
> +
> + if (ssp == NULL)
> + return 0;
> +
> + rc = smk_curacc(ssp, MAY_READWRITE);
> + return rc;
> +}
> +
> +static int smack_shm_shmat(struct shmid_kernel *shp, char __user *shmaddr,
> int shmflg)
> +{
> + smack_t *ssp = smack_of_shm(shp);
> + int rc;
> +
> + if (ssp == NULL)
> + return 0;
> +
> + rc = smk_curacc(ssp, MAY_READWRITE);
> + return rc;
> +}
> +static int smack_sem_associate(struct sem_array *sma, int semflg)
> +{
> + smack_t *ssp = smack_of_sem(sma);
> + int rc;
> +
> + if (ssp == NULL)
> + return 0;
> +
> + rc = smk_curacc(ssp, MAY_READWRITE);
> + return rc;
> +}
> +
> +static int smack_sem_semctl(struct sem_array *sma, int cmd)
> +{
> + smack_t *ssp = smack_of_sem(sma);
> + int rc;
> +
> + if (ssp == NULL)
> + return 0;
> +
> + rc = smk_curacc(ssp, MAY_READWRITE);
> + return rc;
> +}
> +
> +static int smack_sem_semop(struct sem_array *sma, struct sembuf *sops,
> unsigned nsops, int alter)
> +{
> + smack_t *ssp = smack_of_sem(sma);
> + int rc;
> +
> + if (ssp == NULL)
> + return 0;
> +
> + rc = smk_curacc(ssp, MAY_READWRITE);
> + return rc;
> +}
No read-only or write-only sem mappings?
> +static int smack_msg_queue_associate(struct msg_queue *msq, int msqflg)
> +{
> + smack_t *msp = smack_of_msq(msq);
> + int rc;
> +
> + if (msp == NULL)
> + return 0;
> +
> + rc = smk_curacc(msp, MAY_READWRITE);
> + return rc;
> +}
> +
> +static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> +{
> + smack_t *msp = smack_of_msq(msq);
> + int rc;
> +
> + if (msp == NULL)
> + return 0;
> +
> + rc = smk_curacc(msp, MAY_READWRITE);
> + return rc;
> +}
> +
> +static int smack_msg_queue_msgsnd(struct msg_queue *msq, struct msg_msg
> *msg, int msqflg)
> +{
> + smack_t *msp = smack_of_msq(msq);
> + int rc;
> +
> + if (msp == NULL)
> + return 0;
> +
> + rc = smk_curacc(msp, MAY_READWRITE);
> + return rc;
> +}
> +
> +static int smack_msg_queue_msgrcv(struct msg_queue *msq, struct msg_msg
> *msg, struct task_struct *target, long type, int mode)
> +{
> + smack_t *msp = smack_of_msq(msq);
> + int rc;
> +
> + if (msp == NULL)
> + return 0;
> +
> + rc = smk_curacc(msp, MAY_READWRITE);
> + return rc;
> +}
No read-only or write-only msg mappings?
> +static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
> +{
> + smack_t *tsp = smk_of_task(p);
> + smack_t *isp = smk_of_inode(inode);
> +
> + *isp = *tsp;
> +}
Interesting, I read this to myself as "[set] task to inode", rather than
"[give] task's label to inode".
> +static int smack_task_wait(struct task_struct *p)
> +{
> + smack_t *tsp = smk_of_task(p);
> + smack_t *csp = smk_of_task(current);
> + int rc;
> +
> + rc = smk_access(csp, tsp, MAY_WRITE);
No smk_curacc()?
> + if (rc == 0)
> + return 0;
> +
> + /*
> + * Allow the operation to succeed if either task
> + * has privilege to perform operations that might
> + * account for the smack labels having gotten to
> + * be different in the first place.
> + *
> + * This breaks the strict subjet/object access
> + * control ideal, taking the object's privilege
> + * state into account in the decision as well as
> + * the smack value.
> + */
> + if (__capable(current, CAP_MAC_OVERRIDE) ||
> + __capable(p, CAP_MAC_OVERRIDE))
> + return 0;
This worries me a little. Just because the object _could_ have changed
labels doesn't mean it _did_ change labels..
> + return rc;
> +}
Thanks Casey
pgpLJO8yeVrPv.pgp
Description: PGP signature
