Re: Some feedback on the hooks

From: Stephen Smalley (sdsat_private)
Date: Thu Apr 26 2001 - 11:16:16 PDT

  • Next message: Chris Wright: "Re: Some feedback on the hooks"

    > Does the movement of the hook now work for all your cases, or where do
    > you suggest to place a new one, right before/after DQUOT_OFF?  And if
    > the umount fails later after this call, do you need to put things back
    > again?
    
    > I moved the sys_mount check to do_remount.  Since you mention
    > do_remount, should we also hook there?  Actually if you could give a
    > small patch snippet of what you are looking for here, would help me out
    > lots.
    
    I've attached a patch relative to your latest patch that adjusts
    the existing mount hook and adds some additional hooks for the
    mount-related operations based on the SELinux insertion points.  In
    addition to modifying fs/super.c and include/linux/security.h,
    the patch updates kernel/security.c and kernel/capability_plug.c
    appropriately.  In addition to these changes, I would still
    like to see a security field added, preferably to struct
    super_block, but alternatively to struct vfsmount if you prefer.
    Of course, that will require a security operations vector
    with alloc_security and free_security hooks.
    
    On a side note, should the alloc_security and free_security hooks
    be made consistent with the other hooks, i.e. passing a pointer
    to the object and letting the hook set/free the internal security
    field rather than returning/passing the security field itself?
    You already have a case where you must pass the object to the
    alloc_security hook (for linux_binprm), and it seems like
    you might also want the object in inode free_security calls 
    so that you can clear the inode's entry in the persistent
    label mapping.  However, in that case, the inode's free_security
    hook shouldn't be called until just before calling delete in
    iput, so perhaps you want a separate hook for that purpose.
    
    --
    Stephen D. Smalley, NAI Labs
    ssmalleyat_private
    
    
    
    
    
    
    ===== fs/super.c 1.13 vs edited =====
    --- 1.13/fs/super.c	Wed Apr 25 21:14:07 2001
    +++ edited/fs/super.c	Thu Apr 26 13:43:31 2001
    @@ -1066,6 +1066,7 @@
     	 */
     	DQUOT_OFF(sb);
     	acct_auto_close(sb->s_dev);
    +	security_ops->umount_close(mnt);
     
     	/*
     	 * If we may have to abort operations to get out of this
    @@ -1090,6 +1091,7 @@
     	fsync_dev(sb->s_dev);
     
     	if (sb->s_root->d_inode->i_state) {
    +	        security_ops->umount_busy(mnt);
     		mntput(mnt);
     		return -EBUSY;
     	}
    @@ -1099,6 +1101,7 @@
     	spin_lock(&dcache_lock);
     	if (atomic_read(&mnt->mnt_count) > 2) {
     		spin_unlock(&dcache_lock);
    +	        security_ops->umount_busy(mnt);
     		mntput(mnt);
     		return -EBUSY;
     	}
    @@ -1257,11 +1260,18 @@
     				/*
     				 * Shrink the dcache and sync the device.
     				 */
    +				retval = security_ops->remount(nd.mnt, flags, data);
    +				if (retval) {
    +					path_release(&nd);
    +					return retval;
    +				}
     				shrink_dcache_sb(sb);
     				fsync_dev(sb->s_dev);
     				if (flags & MS_RDONLY)
     					acct_auto_close(sb->s_dev);
     				retval = do_remount_sb(sb, flags, data);
    +				if (!retval)
    +					security_ops->post_remount(nd.mnt, flags, data);
     			}
     		}
     		path_release(&nd);
    @@ -1323,11 +1333,6 @@
     	struct super_block *sb;
     	int retval = 0;
     
    -	/* check that we have permission to do this */
    -	retval = security_ops->mount(dev_name, dir_name, type_page, flags, data_page);
    -	if (retval)
    -		return retval;
    -
     	/* Discard magic */
     	if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
     		flags &= ~MS_MGC_MSK;
    @@ -1378,6 +1383,10 @@
     	if (retval)
     		goto fs_out;
     
    +	retval = security_ops->mount(dev_name, &nd, type_page, flags, data_page);
    +	if (retval)
    +		goto fs_out;
    +
     	/* get superblock, locks mount_sem on success */
     	if (fstype->fs_flags & FS_NOMOUNT)
     		sb = ERR_PTR(-EINVAL);
    @@ -1407,6 +1416,11 @@
     		goto fail;
     	down(&nd.dentry->d_inode->i_zombie);
     	if (!IS_DEADDIR(nd.dentry->d_inode)) {
    +		retval = security_ops->add_vfsmnt(&nd, sb, dev_name);
    +		if (retval) {
    +			up(&nd.dentry->d_inode->i_zombie);
    +			goto fail;
    +		}
     		retval = -ENOMEM;
     		mnt = add_vfsmnt(&nd, sb->s_root, dev_name);
     	}
    ===== include/linux/security.h 1.16 vs edited =====
    --- 1.16/include/linux/security.h	Wed Apr 25 21:14:07 2001
    +++ edited/include/linux/security.h	Thu Apr 26 13:44:28 2001
    @@ -123,10 +123,15 @@
     	int  (* sethostname)		(void);		// CAP_SYS_ADMIN
     	int  (* setdomainname)		(void);		// CAP_SYS_ADMIN
     	int  (* reboot)			(unsigned int cmd);	// CAP_SYS_BOOT
    -	int  (* mount)			(char * dev_name, char * dir_name, 
    +        int  (* mount)			(char * dev_name, struct nameidata *nd,
     					 char * type, unsigned long flags, 
     					 void * data);			// part of CAP_SYS_ADMIN
    +        int  (* add_vfsmnt)		        (struct nameidata *nd, struct super_block *sb, char *dev_name);
     	int  (* umount)			(struct vfsmount *mnt, int umount_root, int flags);	// part of CAP_SYS_ADMIN
    +	void  (* umount_close)			(struct vfsmount *mnt);    
    +	void  (* umount_busy)			(struct vfsmount *mnt);    
    +	int   (* remount)			(struct vfsmount *mnt, unsigned long flags, void *data);    
    +	void  (* post_remount)			(struct vfsmount *mnt, unsigned long flags, void *data);    
     	int  (* ioperm)			(void);		// part of CAP_RAWIO
     	int  (* iopl)			(void);		// part of CAP_RAWIO
     	int  (* ptrace)			(void);		// CAP_PTRACE
    ===== kernel/security.c 1.13 vs edited =====
    --- 1.13/kernel/security.c	Wed Apr 25 21:14:07 2001
    +++ edited/kernel/security.c	Thu Apr 26 13:59:40 2001
    @@ -35,8 +35,13 @@
     static int dummy_sethostname	(void)	{return 0;}
     static int dummy_setdomainname	(void)	{return 0;}
     static int dummy_reboot		(unsigned int cmd)	{return 0;}
    -static int dummy_mount		(char * dev_name, char * dir_name, char * type, unsigned long flags, void * data)	{return 0;}
    +static int dummy_mount		(char * dev_name, struct nameidata *nd, char * type, unsigned long flags, void * data)	{return 0;}
    +static int dummy_add_vfsmnt	(struct nameidata *nd, struct super_block *sb, char * dev_name)	{return 0;}
     static int dummy_umount		(struct vfsmount *mnt, int umount_root, int flags)					{return 0;}
    +static void dummy_umount_close	(struct vfsmount *mnt) {return;}
    +static void dummy_umount_busy	(struct vfsmount *mnt) {return;}
    +static int dummy_remount	(struct vfsmount *mnt, unsigned long flags, void *data)					{return 0;}
    +static void dummy_post_remount	(struct vfsmount *mnt, unsigned long flags, void *data)					{return;}
     static int dummy_ioperm		(void)	{return 0;}
     static int dummy_iopl		(void)	{return 0;}
     static int dummy_ptrace		(void)	{return 0;}
    @@ -198,7 +203,12 @@
     	setdomainname:		dummy_setdomainname,
     	reboot:			dummy_reboot,
     	mount:			dummy_mount,
    +	add_vfsmnt:             dummy_add_vfsmnt,
     	umount:			dummy_umount,
    +	umount_close:		dummy_umount_close,
    +	umount_busy:		dummy_umount_busy,
    +	remount:		dummy_remount,
    +	post_remount:		dummy_post_remount,
     	ioperm:			dummy_ioperm,
     	iopl:			dummy_iopl,
     	ptrace:			dummy_ptrace,
    ===== kernel/capability_plug.c 1.6 vs edited =====
    --- 1.6/kernel/capability_plug.c	Wed Apr 25 21:14:07 2001
    +++ edited/kernel/capability_plug.c	Thu Apr 26 14:02:18 2001
    @@ -42,14 +42,15 @@
     {
     	return is_capable(CAP_SYS_BOOT) ? 0 : -EPERM;
     }
    -static int cap_mount(char * dev_name, char * dir_name, char * type, unsigned long flags, void * data)
    -{
    -	return 0;
    -}
    -static int cap_umount(struct vfsmount *mnt, int umount_root, int flags)
    -{
    -	return 0;
    -}
    +
    +static int cap_mount		(char * dev_name, struct nameidata *nd, char * type, unsigned long flags, void * data)	{return 0;}
    +static int cap_add_vfsmnt	(struct nameidata *nd, struct super_block *sb, char * dev_name)	{return 0;}
    +static int cap_umount		(struct vfsmount *mnt, int umount_root, int flags)					{return 0;}
    +static void cap_umount_close	(struct vfsmount *mnt) {return;}
    +static void cap_umount_busy	(struct vfsmount *mnt) {return;}
    +static int cap_remount	(struct vfsmount *mnt, unsigned long flags, void *data)					{return 0;}
    +static void cap_post_remount	(struct vfsmount *mnt, unsigned long flags, void *data)					{return;}
    +
     static int cap_ioperm(void)
     {
     	return 0;
    @@ -505,7 +506,12 @@
     	setdomainname:		cap_setdomainname,
     	reboot:			cap_reboot,
     	mount:			cap_mount,
    +	add_vfsmnt:             cap_add_vfsmnt,
     	umount:			cap_umount,
    +	umount_close:		cap_umount_close,
    +	umount_busy:		cap_umount_busy,
    +	remount:		cap_remount,
    +	post_remount:		cap_post_remount,
     	ioperm:			cap_ioperm,
     	iopl:			cap_iopl,
     	ptrace:			cap_ptrace,
    
    _______________________________________________
    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 : Thu Apr 26 2001 - 11:21:49 PDT