forked from mirrors/linux
		
	selinux: Convert isec->lock into a spinlock
Convert isec->lock from a mutex into a spinlock. Instead of holding the lock while sleeping in inode_doinit_with_dentry, set isec->initialized to LABEL_PENDING and release the lock. Then, when the sid has been determined, re-acquire the lock. If isec->initialized is still set to LABEL_PENDING, set isec->sid; otherwise, the sid has been set by another task (LABEL_INITIALIZED) or invalidated (LABEL_INVALID) in the meantime. This fixes a deadlock on gfs2 where * one task is in inode_doinit_with_dentry -> gfs2_getxattr, holds isec->lock, and tries to acquire the inode's glock, and * another task is in do_xmote -> inode_go_inval -> selinux_inode_invalidate_secctx, holds the inode's glock, and tries to acquire isec->lock. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> [PM: minor tweaks to keep checkpatch.pl happy] Signed-off-by: Paul Moore <paul@paul-moore.com>
This commit is contained in:
		
							parent
							
								
									3322d0d64f
								
							
						
					
					
						commit
						9287aed2ad
					
				
					 2 changed files with 66 additions and 40 deletions
				
			
		|  | @ -231,7 +231,7 @@ static int inode_alloc_security(struct inode *inode) | ||||||
| 	if (!isec) | 	if (!isec) | ||||||
| 		return -ENOMEM; | 		return -ENOMEM; | ||||||
| 
 | 
 | ||||||
| 	mutex_init(&isec->lock); | 	spin_lock_init(&isec->lock); | ||||||
| 	INIT_LIST_HEAD(&isec->list); | 	INIT_LIST_HEAD(&isec->list); | ||||||
| 	isec->inode = inode; | 	isec->inode = inode; | ||||||
| 	isec->sid = SECINITSID_UNLABELED; | 	isec->sid = SECINITSID_UNLABELED; | ||||||
|  | @ -1382,7 +1382,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| { | { | ||||||
| 	struct superblock_security_struct *sbsec = NULL; | 	struct superblock_security_struct *sbsec = NULL; | ||||||
| 	struct inode_security_struct *isec = inode->i_security; | 	struct inode_security_struct *isec = inode->i_security; | ||||||
| 	u32 sid; | 	u32 task_sid, sid = 0; | ||||||
|  | 	u16 sclass; | ||||||
| 	struct dentry *dentry; | 	struct dentry *dentry; | ||||||
| #define INITCONTEXTLEN 255 | #define INITCONTEXTLEN 255 | ||||||
| 	char *context = NULL; | 	char *context = NULL; | ||||||
|  | @ -1392,7 +1393,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 	if (isec->initialized == LABEL_INITIALIZED) | 	if (isec->initialized == LABEL_INITIALIZED) | ||||||
| 		return 0; | 		return 0; | ||||||
| 
 | 
 | ||||||
| 	mutex_lock(&isec->lock); | 	spin_lock(&isec->lock); | ||||||
| 	if (isec->initialized == LABEL_INITIALIZED) | 	if (isec->initialized == LABEL_INITIALIZED) | ||||||
| 		goto out_unlock; | 		goto out_unlock; | ||||||
| 
 | 
 | ||||||
|  | @ -1411,12 +1412,18 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 		goto out_unlock; | 		goto out_unlock; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	sclass = isec->sclass; | ||||||
|  | 	task_sid = isec->task_sid; | ||||||
|  | 	sid = isec->sid; | ||||||
|  | 	isec->initialized = LABEL_PENDING; | ||||||
|  | 	spin_unlock(&isec->lock); | ||||||
|  | 
 | ||||||
| 	switch (sbsec->behavior) { | 	switch (sbsec->behavior) { | ||||||
| 	case SECURITY_FS_USE_NATIVE: | 	case SECURITY_FS_USE_NATIVE: | ||||||
| 		break; | 		break; | ||||||
| 	case SECURITY_FS_USE_XATTR: | 	case SECURITY_FS_USE_XATTR: | ||||||
| 		if (!(inode->i_opflags & IOP_XATTR)) { | 		if (!(inode->i_opflags & IOP_XATTR)) { | ||||||
| 			isec->sid = sbsec->def_sid; | 			sid = sbsec->def_sid; | ||||||
| 			break; | 			break; | ||||||
| 		} | 		} | ||||||
| 		/* Need a dentry, since the xattr API requires one.
 | 		/* Need a dentry, since the xattr API requires one.
 | ||||||
|  | @ -1438,7 +1445,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 			 * inode_doinit with a dentry, before these inodes could | 			 * inode_doinit with a dentry, before these inodes could | ||||||
| 			 * be used again by userspace. | 			 * be used again by userspace. | ||||||
| 			 */ | 			 */ | ||||||
| 			goto out_unlock; | 			goto out; | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| 		len = INITCONTEXTLEN; | 		len = INITCONTEXTLEN; | ||||||
|  | @ -1446,7 +1453,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 		if (!context) { | 		if (!context) { | ||||||
| 			rc = -ENOMEM; | 			rc = -ENOMEM; | ||||||
| 			dput(dentry); | 			dput(dentry); | ||||||
| 			goto out_unlock; | 			goto out; | ||||||
| 		} | 		} | ||||||
| 		context[len] = '\0'; | 		context[len] = '\0'; | ||||||
| 		rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); | 		rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); | ||||||
|  | @ -1457,14 +1464,14 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 			rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0); | 			rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0); | ||||||
| 			if (rc < 0) { | 			if (rc < 0) { | ||||||
| 				dput(dentry); | 				dput(dentry); | ||||||
| 				goto out_unlock; | 				goto out; | ||||||
| 			} | 			} | ||||||
| 			len = rc; | 			len = rc; | ||||||
| 			context = kmalloc(len+1, GFP_NOFS); | 			context = kmalloc(len+1, GFP_NOFS); | ||||||
| 			if (!context) { | 			if (!context) { | ||||||
| 				rc = -ENOMEM; | 				rc = -ENOMEM; | ||||||
| 				dput(dentry); | 				dput(dentry); | ||||||
| 				goto out_unlock; | 				goto out; | ||||||
| 			} | 			} | ||||||
| 			context[len] = '\0'; | 			context[len] = '\0'; | ||||||
| 			rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); | 			rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); | ||||||
|  | @ -1476,7 +1483,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 				       "%d for dev=%s ino=%ld\n", __func__, | 				       "%d for dev=%s ino=%ld\n", __func__, | ||||||
| 				       -rc, inode->i_sb->s_id, inode->i_ino); | 				       -rc, inode->i_sb->s_id, inode->i_ino); | ||||||
| 				kfree(context); | 				kfree(context); | ||||||
| 				goto out_unlock; | 				goto out; | ||||||
| 			} | 			} | ||||||
| 			/* Map ENODATA to the default file SID */ | 			/* Map ENODATA to the default file SID */ | ||||||
| 			sid = sbsec->def_sid; | 			sid = sbsec->def_sid; | ||||||
|  | @ -1506,28 +1513,25 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 			} | 			} | ||||||
| 		} | 		} | ||||||
| 		kfree(context); | 		kfree(context); | ||||||
| 		isec->sid = sid; |  | ||||||
| 		break; | 		break; | ||||||
| 	case SECURITY_FS_USE_TASK: | 	case SECURITY_FS_USE_TASK: | ||||||
| 		isec->sid = isec->task_sid; | 		sid = task_sid; | ||||||
| 		break; | 		break; | ||||||
| 	case SECURITY_FS_USE_TRANS: | 	case SECURITY_FS_USE_TRANS: | ||||||
| 		/* Default to the fs SID. */ | 		/* Default to the fs SID. */ | ||||||
| 		isec->sid = sbsec->sid; | 		sid = sbsec->sid; | ||||||
| 
 | 
 | ||||||
| 		/* Try to obtain a transition SID. */ | 		/* Try to obtain a transition SID. */ | ||||||
| 		rc = security_transition_sid(isec->task_sid, sbsec->sid, | 		rc = security_transition_sid(task_sid, sid, sclass, NULL, &sid); | ||||||
| 					     isec->sclass, NULL, &sid); |  | ||||||
| 		if (rc) | 		if (rc) | ||||||
| 			goto out_unlock; | 			goto out; | ||||||
| 		isec->sid = sid; |  | ||||||
| 		break; | 		break; | ||||||
| 	case SECURITY_FS_USE_MNTPOINT: | 	case SECURITY_FS_USE_MNTPOINT: | ||||||
| 		isec->sid = sbsec->mntpoint_sid; | 		sid = sbsec->mntpoint_sid; | ||||||
| 		break; | 		break; | ||||||
| 	default: | 	default: | ||||||
| 		/* Default to the fs superblock SID. */ | 		/* Default to the fs superblock SID. */ | ||||||
| 		isec->sid = sbsec->sid; | 		sid = sbsec->sid; | ||||||
| 
 | 
 | ||||||
| 		if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) { | 		if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) { | ||||||
| 			/* We must have a dentry to determine the label on
 | 			/* We must have a dentry to determine the label on
 | ||||||
|  | @ -1550,21 +1554,30 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent | ||||||
| 			 * could be used again by userspace. | 			 * could be used again by userspace. | ||||||
| 			 */ | 			 */ | ||||||
| 			if (!dentry) | 			if (!dentry) | ||||||
| 				goto out_unlock; | 				goto out; | ||||||
| 			rc = selinux_genfs_get_sid(dentry, isec->sclass, | 			rc = selinux_genfs_get_sid(dentry, sclass, | ||||||
| 						   sbsec->flags, &sid); | 						   sbsec->flags, &sid); | ||||||
| 			dput(dentry); | 			dput(dentry); | ||||||
| 			if (rc) | 			if (rc) | ||||||
| 				goto out_unlock; | 				goto out; | ||||||
| 			isec->sid = sid; |  | ||||||
| 		} | 		} | ||||||
| 		break; | 		break; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | out: | ||||||
|  | 	spin_lock(&isec->lock); | ||||||
|  | 	if (isec->initialized == LABEL_PENDING) { | ||||||
|  | 		if (!sid || rc) { | ||||||
|  | 			isec->initialized = LABEL_INVALID; | ||||||
|  | 			goto out_unlock; | ||||||
|  | 		} | ||||||
|  | 
 | ||||||
| 		isec->initialized = LABEL_INITIALIZED; | 		isec->initialized = LABEL_INITIALIZED; | ||||||
|  | 		isec->sid = sid; | ||||||
|  | 	} | ||||||
| 
 | 
 | ||||||
| out_unlock: | out_unlock: | ||||||
| 	mutex_unlock(&isec->lock); | 	spin_unlock(&isec->lock); | ||||||
| 	return rc; | 	return rc; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -3195,9 +3208,11 @@ static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name, | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	isec = backing_inode_security(dentry); | 	isec = backing_inode_security(dentry); | ||||||
|  | 	spin_lock(&isec->lock); | ||||||
| 	isec->sclass = inode_mode_to_security_class(inode->i_mode); | 	isec->sclass = inode_mode_to_security_class(inode->i_mode); | ||||||
| 	isec->sid = newsid; | 	isec->sid = newsid; | ||||||
| 	isec->initialized = LABEL_INITIALIZED; | 	isec->initialized = LABEL_INITIALIZED; | ||||||
|  | 	spin_unlock(&isec->lock); | ||||||
| 
 | 
 | ||||||
| 	return; | 	return; | ||||||
| } | } | ||||||
|  | @ -3290,9 +3305,11 @@ static int selinux_inode_setsecurity(struct inode *inode, const char *name, | ||||||
| 	if (rc) | 	if (rc) | ||||||
| 		return rc; | 		return rc; | ||||||
| 
 | 
 | ||||||
|  | 	spin_lock(&isec->lock); | ||||||
| 	isec->sclass = inode_mode_to_security_class(inode->i_mode); | 	isec->sclass = inode_mode_to_security_class(inode->i_mode); | ||||||
| 	isec->sid = newsid; | 	isec->sid = newsid; | ||||||
| 	isec->initialized = LABEL_INITIALIZED; | 	isec->initialized = LABEL_INITIALIZED; | ||||||
|  | 	spin_unlock(&isec->lock); | ||||||
| 	return 0; | 	return 0; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -3953,9 +3970,11 @@ static void selinux_task_to_inode(struct task_struct *p, | ||||||
| 	struct inode_security_struct *isec = inode->i_security; | 	struct inode_security_struct *isec = inode->i_security; | ||||||
| 	u32 sid = task_sid(p); | 	u32 sid = task_sid(p); | ||||||
| 
 | 
 | ||||||
|  | 	spin_lock(&isec->lock); | ||||||
| 	isec->sclass = inode_mode_to_security_class(inode->i_mode); | 	isec->sclass = inode_mode_to_security_class(inode->i_mode); | ||||||
| 	isec->sid = sid; | 	isec->sid = sid; | ||||||
| 	isec->initialized = LABEL_INITIALIZED; | 	isec->initialized = LABEL_INITIALIZED; | ||||||
|  | 	spin_unlock(&isec->lock); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /* Returns error only if unable to parse addresses */ | /* Returns error only if unable to parse addresses */ | ||||||
|  | @ -4274,24 +4293,24 @@ static int selinux_socket_post_create(struct socket *sock, int family, | ||||||
| 	const struct task_security_struct *tsec = current_security(); | 	const struct task_security_struct *tsec = current_security(); | ||||||
| 	struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(sock)); | 	struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(sock)); | ||||||
| 	struct sk_security_struct *sksec; | 	struct sk_security_struct *sksec; | ||||||
|  | 	u16 sclass = socket_type_to_security_class(family, type, protocol); | ||||||
|  | 	u32 sid = SECINITSID_KERNEL; | ||||||
| 	int err = 0; | 	int err = 0; | ||||||
| 
 | 
 | ||||||
| 	isec->sclass = socket_type_to_security_class(family, type, protocol); | 	if (!kern) { | ||||||
| 
 | 		err = socket_sockcreate_sid(tsec, sclass, &sid); | ||||||
| 	if (kern) |  | ||||||
| 		isec->sid = SECINITSID_KERNEL; |  | ||||||
| 	else { |  | ||||||
| 		err = socket_sockcreate_sid(tsec, isec->sclass, &(isec->sid)); |  | ||||||
| 		if (err) | 		if (err) | ||||||
| 			return err; | 			return err; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	isec->sclass = sclass; | ||||||
|  | 	isec->sid = sid; | ||||||
| 	isec->initialized = LABEL_INITIALIZED; | 	isec->initialized = LABEL_INITIALIZED; | ||||||
| 
 | 
 | ||||||
| 	if (sock->sk) { | 	if (sock->sk) { | ||||||
| 		sksec = sock->sk->sk_security; | 		sksec = sock->sk->sk_security; | ||||||
| 		sksec->sid = isec->sid; | 		sksec->sclass = sclass; | ||||||
| 		sksec->sclass = isec->sclass; | 		sksec->sid = sid; | ||||||
| 		err = selinux_netlbl_socket_post_create(sock->sk, family); | 		err = selinux_netlbl_socket_post_create(sock->sk, family); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | @ -4467,16 +4486,22 @@ static int selinux_socket_accept(struct socket *sock, struct socket *newsock) | ||||||
| 	int err; | 	int err; | ||||||
| 	struct inode_security_struct *isec; | 	struct inode_security_struct *isec; | ||||||
| 	struct inode_security_struct *newisec; | 	struct inode_security_struct *newisec; | ||||||
|  | 	u16 sclass; | ||||||
|  | 	u32 sid; | ||||||
| 
 | 
 | ||||||
| 	err = sock_has_perm(current, sock->sk, SOCKET__ACCEPT); | 	err = sock_has_perm(current, sock->sk, SOCKET__ACCEPT); | ||||||
| 	if (err) | 	if (err) | ||||||
| 		return err; | 		return err; | ||||||
| 
 | 
 | ||||||
| 	newisec = inode_security_novalidate(SOCK_INODE(newsock)); |  | ||||||
| 
 |  | ||||||
| 	isec = inode_security_novalidate(SOCK_INODE(sock)); | 	isec = inode_security_novalidate(SOCK_INODE(sock)); | ||||||
| 	newisec->sclass = isec->sclass; | 	spin_lock(&isec->lock); | ||||||
| 	newisec->sid = isec->sid; | 	sclass = isec->sclass; | ||||||
|  | 	sid = isec->sid; | ||||||
|  | 	spin_unlock(&isec->lock); | ||||||
|  | 
 | ||||||
|  | 	newisec = inode_security_novalidate(SOCK_INODE(newsock)); | ||||||
|  | 	newisec->sclass = sclass; | ||||||
|  | 	newisec->sid = sid; | ||||||
| 	newisec->initialized = LABEL_INITIALIZED; | 	newisec->initialized = LABEL_INITIALIZED; | ||||||
| 
 | 
 | ||||||
| 	return 0; | 	return 0; | ||||||
|  | @ -5979,9 +6004,9 @@ static void selinux_inode_invalidate_secctx(struct inode *inode) | ||||||
| { | { | ||||||
| 	struct inode_security_struct *isec = inode->i_security; | 	struct inode_security_struct *isec = inode->i_security; | ||||||
| 
 | 
 | ||||||
| 	mutex_lock(&isec->lock); | 	spin_lock(&isec->lock); | ||||||
| 	isec->initialized = LABEL_INVALID; | 	isec->initialized = LABEL_INVALID; | ||||||
| 	mutex_unlock(&isec->lock); | 	spin_unlock(&isec->lock); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /*
 | /*
 | ||||||
|  |  | ||||||
|  | @ -39,7 +39,8 @@ struct task_security_struct { | ||||||
| 
 | 
 | ||||||
| enum label_initialized { | enum label_initialized { | ||||||
| 	LABEL_INVALID,		/* invalid or not initialized */ | 	LABEL_INVALID,		/* invalid or not initialized */ | ||||||
| 	LABEL_INITIALIZED	/* initialized */ | 	LABEL_INITIALIZED,	/* initialized */ | ||||||
|  | 	LABEL_PENDING | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| struct inode_security_struct { | struct inode_security_struct { | ||||||
|  | @ -52,7 +53,7 @@ struct inode_security_struct { | ||||||
| 	u32 sid;		/* SID of this object */ | 	u32 sid;		/* SID of this object */ | ||||||
| 	u16 sclass;		/* security class of this object */ | 	u16 sclass;		/* security class of this object */ | ||||||
| 	unsigned char initialized;	/* initialization flag */ | 	unsigned char initialized;	/* initialization flag */ | ||||||
| 	struct mutex lock; | 	spinlock_t lock; | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| struct file_security_struct { | struct file_security_struct { | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Andreas Gruenbacher
						Andreas Gruenbacher