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:
Andreas Gruenbacher 2016-11-15 11:06:40 +01:00 committed by Paul Moore
parent 3322d0d64f
commit 9287aed2ad
2 changed files with 66 additions and 40 deletions

View File

@ -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;
} }
isec->initialized = LABEL_INITIALIZED; 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->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);
} }
/* /*

View File

@ -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 {