I've written a patch (attached) to move the check_sb and post_addmount hooks from graft_tree into attach_mnt. If we were to actually want to do this, then we probably want to rename these hooks. Please notice the need for a new fs/namespace.c function (undo_detach_mnt, which could be done by adding a flag to attach_mnt to skip security checks). Do people feel this is worth doing? The codepaths which an attach_mnt hook catch, which graft_tree does not, are: 1. copy_tree: The validity of the mountpoints being copied were already checked at prior mount. (In DTE's case, these checks would simply return immediately, as the superblock will already have been set up.) 2. do_movemount. Currently protected by sb_mount. 3. pivot_root: Currently protected by sb_pivotroot. If it's worth doing, then I'll go ahead and rename the hooks and finalize the patch. thanks, -serge diff -r -U 10 orig/lsm-2.5/fs/namespace.c lsm-2.5/fs/namespace.c --- orig/lsm-2.5/fs/namespace.c Sun Sep 29 22:04:22 2002 +++ lsm-2.5/fs/namespace.c Sun Sep 29 18:37:09 2002 @@ -98,29 +98,43 @@ { old_nd->dentry = mnt->mnt_mountpoint; old_nd->mnt = mnt->mnt_parent; mnt->mnt_parent = mnt; mnt->mnt_mountpoint = mnt->mnt_root; list_del_init(&mnt->mnt_child); list_del_init(&mnt->mnt_hash); old_nd->dentry->d_mounted--; } -static void attach_mnt(struct vfsmount *mnt, struct nameidata *nd) +static void undo_detach_mnt(struct vfsmount *mnt, struct nameidata *nd) { mnt->mnt_parent = mntget(nd->mnt); mnt->mnt_mountpoint = dget(nd->dentry); list_add(&mnt->mnt_hash, mount_hashtable+hash(nd->mnt, nd->dentry)); list_add(&mnt->mnt_child, &nd->mnt->mnt_mounts); nd->dentry->d_mounted++; } +static int attach_mnt(struct vfsmount *mnt, struct nameidata *nd) +{ + int err = security_ops->sb_check_sb(mnt, nd); + if (err) + return err; + mnt->mnt_parent = mntget(nd->mnt); + mnt->mnt_mountpoint = dget(nd->dentry); + list_add(&mnt->mnt_hash, mount_hashtable+hash(nd->mnt, nd->dentry)); + list_add(&mnt->mnt_child, &nd->mnt->mnt_mounts); + nd->dentry->d_mounted++; + security_ops->sb_post_addmount(mnt, nd); + return 0; +} + static struct vfsmount *next_mnt(struct vfsmount *p, struct vfsmount *root) { struct list_head *next = p->mnt_mounts.next; if (next == &p->mnt_mounts) { while (1) { if (p == root) return NULL; next = p->mnt_child.next; if (next != &p->mnt_parent->mnt_mounts) break; @@ -414,20 +428,21 @@ if (permission(nd->dentry->d_inode, MAY_WRITE)) return -EPERM; return 0; #endif } static struct vfsmount *copy_tree(struct vfsmount *mnt, struct dentry *dentry) { struct vfsmount *p, *next, *q, *res; struct nameidata nd; + int err; p = mnt; res = nd.mnt = q = clone_mnt(p, dentry); if (!q) goto Enomem; q->mnt_parent = q; q->mnt_mountpoint = p->mnt_mountpoint; while ( (next = next_mnt(p, mnt)) != NULL) { while (p != next->mnt_parent) { @@ -435,24 +450,30 @@ q = q->mnt_parent; } p = next; nd.mnt = q; nd.dentry = p->mnt_mountpoint; q = clone_mnt(p, p->mnt_root); if (!q) goto Enomem; spin_lock(&dcache_lock); list_add_tail(&q->mnt_list, &res->mnt_list); - attach_mnt(q, &nd); + err = attach_mnt(q, &nd); + if (err) + goto out_unlock; spin_unlock(&dcache_lock); } return res; +out_unlock: + list_del(&q->mnt_list); + spin_unlock(&dcache_lock); + mntput(q); Enomem: if (res) { spin_lock(&dcache_lock); umount_tree(res); spin_unlock(&dcache_lock); } return NULL; } static int graft_tree(struct vfsmount *mnt, struct nameidata *nd) @@ -463,38 +484,35 @@ if (S_ISDIR(nd->dentry->d_inode->i_mode) != S_ISDIR(mnt->mnt_root->d_inode->i_mode)) return -ENOTDIR; err = -ENOENT; down(&nd->dentry->d_inode->i_sem); if (IS_DEADDIR(nd->dentry->d_inode)) goto out_unlock; - err = security_ops->sb_check_sb(mnt, nd); - if (err) - goto out_unlock; - spin_lock(&dcache_lock); if (IS_ROOT(nd->dentry) || !d_unhashed(nd->dentry)) { struct list_head head; - attach_mnt(mnt, nd); + err = attach_mnt(mnt, nd); + if (err) + goto out_spin_unlock; list_add_tail(&head, &mnt->mnt_list); list_splice(&head, current->namespace->list.prev); mntget(mnt); err = 0; } +out_spin_unlock: spin_unlock(&dcache_lock); out_unlock: up(&nd->dentry->d_inode->i_sem); - if (!err) - security_ops->sb_post_addmount(mnt, nd); return err; } /* * do loopback mount. */ static int do_loopback(struct nameidata *nd, char *old_name, int recurse) { struct nameidata old_nd; struct vfsmount *mnt = NULL; @@ -602,21 +620,23 @@ S_ISDIR(old_nd.dentry->d_inode->i_mode)) goto out2; err = -ELOOP; for (p = nd->mnt; p->mnt_parent!=p; p = p->mnt_parent) if (p == old_nd.mnt) goto out2; err = 0; detach_mnt(old_nd.mnt, &parent_nd); - attach_mnt(old_nd.mnt, nd); + err = attach_mnt(old_nd.mnt, nd); + if (err) + undo_detach_mnt(old_nd.mnt, &parent_nd); out2: spin_unlock(&dcache_lock); out1: up(&nd->dentry->d_inode->i_sem); out: up_write(¤t->namespace->sem); if (!err) path_release(&parent_nd); path_release(&old_nd); return err; @@ -978,26 +998,37 @@ if (tmp->mnt_parent == new_nd.mnt) break; tmp = tmp->mnt_parent; } if (!is_subdir(tmp->mnt_mountpoint, new_nd.dentry)) goto out3; } else if (!is_subdir(old_nd.dentry, new_nd.dentry)) goto out3; detach_mnt(new_nd.mnt, &parent_nd); detach_mnt(user_nd.mnt, &root_parent); - attach_mnt(user_nd.mnt, &old_nd); - attach_mnt(new_nd.mnt, &root_parent); + error = attach_mnt(user_nd.mnt, &old_nd); + if (error) { + undo_detach_mnt(user_nd.mnt, &root_parent); + undo_detach_mnt(new_nd.mnt, &parent_nd); + } else { + error = attach_mnt(new_nd.mnt, &root_parent); + if (error) { + detach_mnt(user_nd.mnt, &old_nd); + undo_detach_mnt(user_nd.mnt, &root_parent); + undo_detach_mnt(new_nd.mnt, &parent_nd); + } + } spin_unlock(&dcache_lock); - chroot_fs_refs(&user_nd, &new_nd); - security_ops->sb_post_pivotroot(&user_nd, &new_nd); - error = 0; + if (!error) { + chroot_fs_refs(&user_nd, &new_nd); + security_ops->sb_post_pivotroot(&user_nd, &new_nd); + } path_release(&root_parent); path_release(&parent_nd); out2: up(&old_nd.dentry->d_inode->i_sem); up_write(¤t->namespace->sem); path_release(&user_nd); path_release(&old_nd); out1: path_release(&new_nd); out0: diff -r -U 10 orig/lsm-2.5/security/dte/dte.c lsm-2.5/security/dte/dte.c --- orig/lsm-2.5/security/dte/dte.c Sun Sep 29 22:04:33 2002 +++ lsm-2.5/security/dte/dte.c Sun Sep 29 16:19:14 2002 @@ -46,20 +46,21 @@ extern void dte_inode_post_create (struct inode *inode, struct dentry *dentry, int mask); extern int dte_inode_permission (struct inode *inode, int mask); extern int dte_inode_permission_lite(struct inode *inode, int mask); extern int dte_task_alloc_security (struct task_struct *p); extern void dte_task_free_security (struct task_struct *p); extern void dte_post_lookup (struct inode *ino, struct dentry *d); extern int dte_sb_alloc_security (struct super_block *sb); extern void dte_sb_free_security (struct super_block *sb); extern int dte_mount (char * dev_name, struct nameidata *nd, char * type, unsigned long flags, void * data); +extern int dte_sb_kern_mount (struct super_block *sb); extern int dte_umount (struct vfsmount *mnt, int flags); extern int dte_check_sb (struct vfsmount *mnt, struct nameidata *nd); extern void dte_inode_post_mknod (struct inode *inode, struct dentry *dentry, int major, dev_t minor); extern void dte_inode_post_symlink (struct inode *inode, struct dentry *dentry, const char *name); extern void dte_inode_post_mkdir (struct inode *inode, struct dentry *dentry, int mask); extern int dte_task_kill (struct task_struct *p, struct siginfo *info, int sig); @@ -227,25 +228,20 @@ { if (dte_secondary_ops) dte_secondary_ops->bprm_compute_creds(bprm); } static int dte_binprm_check_security (struct linux_binprm *bprm) { return 0; } -static int dte_sb_kern_mount (struct super_block *sb) -{ - return 0; -} - static int dte_sb_statfs (struct super_block *sb) { return 0; } static void dte_umount_close (struct vfsmount *mnt) { return; } diff -r -U 10 orig/lsm-2.5/security/dte/dte.h lsm-2.5/security/dte/dte.h --- orig/lsm-2.5/security/dte/dte.h Sun Sep 29 22:04:33 2002 +++ lsm-2.5/security/dte/dte.h Sun Sep 29 18:14:27 2002 @@ -247,21 +247,21 @@ int mask); int dte_inode_permission (struct inode *inode, int mask); int find_type_conv(char **conv_array, int array_size, char *type); /* from mount.c: */ void hierarchical_setup(struct vfsmount *mnt); void dte_post_mountroot (void); void dte_post_addmount (struct vfsmount *mnt, struct nameidata *nd); int dte_sb_alloc_security (struct super_block *sb); void dte_sb_free_security (struct super_block *sb); -void dte_setup_eafile(struct super_block *sb, struct vfsmount *mnt); +void dte_setup_eafile(struct super_block *sb); int dte_mount (char * dev_name, struct nameidata *nd, char * type, unsigned long flags, void * data); int dte_check_sb (struct vfsmount *mnt, struct nameidata *nd); int dte_mount (char * dev_name, struct nameidata *nd, char * type, unsigned long flags, void * data); int dte_umount (struct vfsmount *mnt, int flags); int dte_check_sb (struct vfsmount *mnt, struct nameidata *nd); /* from path.c: */ struct dte_map_node *dte_find_map_node_create(char *path); diff -r -U 10 orig/lsm-2.5/security/dte/module.c lsm-2.5/security/dte/module.c --- orig/lsm-2.5/security/dte/module.c Sun Sep 29 22:04:33 2002 +++ lsm-2.5/security/dte/module.c Sun Sep 29 18:21:40 2002 @@ -35,21 +35,21 @@ mm_segment_t old_fs; /* * check for external attributes file * handle the 'true parent' hookups */ printk(KERN_NOTICE "dte_walk_dcache_tree_full: called on %s.\n", pmnt->mnt_devname); old_fs = get_fs(); set_fs(KERNEL_DS); - dte_setup_eafile(pmnt->mnt_sb, pmnt); + dte_setup_eafile(pmnt->mnt_sb); set_fs(old_fs); hierarchical_setup(pmnt); printk(KERN_NOTICE "dte_walk_dcache_tree_full: did setup on %s.\n", pmnt->mnt_devname); repeat: next = this_parent->d_subdirs.next; resume: while (next != &this_parent->d_subdirs) { struct list_head *tmp = next; diff -r -U 10 orig/lsm-2.5/security/dte/mount.c lsm-2.5/security/dte/mount.c --- orig/lsm-2.5/security/dte/mount.c Sun Sep 29 22:04:33 2002 +++ lsm-2.5/security/dte/mount.c Sun Sep 29 21:06:35 2002 @@ -94,94 +94,77 @@ } /* * Look for the ea file * It will be in the root directory, named 'dteeaf' * * Note: mnt is null only when called from post_mountroot, in which case * sb->s_root is in fact root, and there's no fallback for deftype other * than the default utype (no parent inode) */ -void dte_setup_eafile(struct super_block *sb, struct vfsmount *mnt) +void dte_setup_eafile(struct super_block *sb) { - char *devname; struct dentry *dentry; char buf[1024], *bufp, *bufp2; struct dte_sb_sec *sb_sec; - struct dte_inode_sec *p; long long offset; int i, err; struct file *fp; if (!dte_initialized) return; sb_sec = sb->s_security; if (!sb_sec) { printk(KERN_ERR "dte_setup_eafile: no s_security on the superblock! (%3d/%3d)\n", MAJOR(sb->s_dev), MINOR(sb->s_dev)); dte_sb_alloc_security(sb); sb_sec = (struct dte_sb_sec *)sb->s_security; } if (sb_sec->initialized) return; - if (mnt) { - p = (struct dte_inode_sec *) - mnt->mnt_mountpoint->d_inode->i_security; - devname = mnt->mnt_devname; - dentry = lookup_one_len("dteeaf", mnt->mnt_root, 6); - err = PTR_ERR(dentry); - if (IS_ERR(dentry) || !dentry->d_inode) { - printk(KERN_NOTICE "dte_setup_eafile: error opening ea file for %s, %d.\n", - devname, err); - return; - } - } else { - devname = "/dev/root"; - dentry = lookup_one_len("dteeaf", sb->s_root, 6); - err = PTR_ERR(dentry); - if (IS_ERR(dentry) || !dentry->d_inode) { - printk(KERN_NOTICE "dte_setup_eafile: error opening ea file for %s, %d.\n", - devname, err); - return; - } + dentry = lookup_one_len("dteeaf", sb->s_root, 6); + err = PTR_ERR(dentry); + if (IS_ERR(dentry) || !dentry->d_inode) { + printk(KERN_NOTICE "dte_setup_eafile: error opening ea file (%d).\n", + err); + return; } err = init_private_file(sb_sec->fp, dentry, FMODE_READ|FMODE_WRITE); fp = sb_sec->fp; fp->f_flags = O_RDWR | O_SYNC; if (err) { - printk(KERN_NOTICE "dte_setup_eafile: no dte ea file for %s, %d.\n", - devname, err); + printk(KERN_NOTICE "dte_setup_eafile: no dte ea file for (%d).\n", + err); } else if (!fp->f_op || !fp->f_op->read || !fp->f_op->write) { - printk(KERN_NOTICE "dte_setup_eafile: no rw support for %s's ea file.\n", - devname); + printk(KERN_NOTICE "dte_setup_eafile: no rw support for ea file.\n"); dput(fp->f_dentry); } else { /* read type table from the ea file */ offset = 0; fp->f_op->read(fp, buf, 1024, &offset); sb_sec->ntypes = xtoi(buf); printk(KERN_NOTICE "dte_setup_eafile: there were %d types, buf %4s.\n", sb_sec->ntypes, buf); sb_sec->type_conv = kmalloc(sb_sec->ntypes*sizeof(char *), GFP_KERNEL); memset(sb_sec->type_conv, 0, sb_sec->ntypes*sizeof(char *)); bufp2 = bufp = buf+4; for (i=0; i<sb_sec->ntypes; i++) { while (*bufp2!='\n') bufp2++; sb_sec->type_conv[i] = dte_get_type(bufp, bufp2); if (!sb_sec->type_conv[i]) { *bufp2 = '\0'; - panic("dev %s: can't find type number %d, %s.\n", - devname, i, bufp); + panic("dte_setup_ea_file: can't find type number %d, %s.\n", + i, bufp); } bufp = ++bufp2; } sb_sec->offset = bufp2-buf; sb_sec->initialized = 1; sb_sec->fp_ready = 1; } printk(KERN_NOTICE "dte_setup_eafile: done\n"); } @@ -305,21 +288,20 @@ goto resume; } } void dte_post_mountroot (void) { struct super_block *sb; struct dte_inode_sec *s; struct dte_task_sec *task_sec; struct task_struct *p; - mm_segment_t old_fs; struct dte_sb_sec *sb_sec; if (read_dte_config()) { panic("Error reading DTE policy!\n"); } if (!current->fs || !current->fs->rootmnt || !current->fs->rootmnt->mnt_sb) { printk(KERN_NOTICE "dte_post_mountroot: no sb\n"); return; } sb = current->fs->rootmnt->mnt_sb; @@ -364,56 +346,41 @@ panic("DTE: NO memory for mount restrictions.\n"); if (!dte_root_mapnode || !dte_root_mapnode->etype || !dte_root_mapnode->utype) panic("Whoa: DTE: no root etype/utype set. Stopping.\n"); #ifdef CONFIG_DTE_VERBOSE show_dte(); #endif - printk(KERN_NOTICE "dte_post_mountroot: almost finished.\n"); dte_initialized = 1; - /* external attribute file setup */ - old_fs = get_fs(); - set_fs(KERNEL_DS); - dte_setup_eafile(sb, NULL); - set_fs(old_fs); printk(KERN_NOTICE "dte_post_mountroot: finished.\n"); } void dte_post_addmount (struct vfsmount *mnt, struct nameidata *nd) { struct dte_inode_sec *p; struct dentry *mntroot = mnt->mnt_root; - mm_segment_t old_fs; if (!dte_initialized) return ; printk(KERN_NOTICE "dte_post_addmount: Called on %s.\n", mnt->mnt_devname); if (!mnt->mnt_root) { printk(KERN_NOTICE "dte_post_addmount: no root dentry for dev %s.\n", mnt->mnt_devname); return; } /* - * check for external attributes file - */ - old_fs = get_fs(); - set_fs(KERNEL_DS); - dte_setup_eafile(mnt->mnt_sb, mnt); - set_fs(old_fs); - - /* * handle the 'true parent' hookups */ hierarchical_setup(mnt); /* * lots of oops checks, but mainly just set the security and type info * on the root inode */ p = (struct dte_inode_sec *)mntroot->d_inode->i_security; if (!p) { @@ -430,20 +397,36 @@ return; } } int dte_mount (char * dev_name, struct nameidata *nd, char * type, unsigned long flags, void * data) { return 0; } +int dte_sb_kern_mount (struct super_block *sb) +{ + mm_segment_t old_fs; + + /* + * check for external attributes file + */ + old_fs = get_fs(); + set_fs(KERNEL_DS); + dte_setup_eafile(sb); + set_fs(old_fs); + if (sb->s_root->d_inode && !sb->s_root->d_inode->i_security) + dte_inode_alloc_security(sb->s_root->d_inode); + return 0; +} + int dte_umount (struct vfsmount *mnt, int flags) { struct super_block *sb = mnt->mnt_sb; struct dte_sb_sec *sb_sec = sb->s_security; printk(KERN_NOTICE "dte_umount: cleaning up ea fp and parents for %s (0).\n", mnt->mnt_devname); down(&sb_sec->s_sem); if (atomic_read(&sb->s_active) != 1) { up(&sb_sec->s_sem); _______________________________________________ linux-security-module mailing list linux-security-moduleat_private http://mail.wirex.com/mailman/listinfo/linux-security-module
This archive was generated by hypermail 2b30 : Sun Sep 29 2002 - 13:55:47 PDT