selinux: move user accesses in selinuxfs out of locked regions
If a user is accessing a file in selinuxfs with a pointer to a userspace
buffer that is backed by e.g. a userfaultfd, the userspace access can
stall indefinitely, which can block fsi->mutex if it is held.
For sel_read_policy(), remove the locking, since this method doesn't seem
to access anything that requires locking.
For sel_read_bool(), move the user access below the locked region.
For sel_write_bool() and sel_commit_bools_write(), move the user access
up above the locked region.
Cc: stable@vger.kernel.org
Fixes: 1da177e4c3
("Linux-2.6.12-rc2")
Signed-off-by: Jann Horn <jannh@google.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
[PM: removed an unused variable in sel_read_policy()]
Signed-off-by: Paul Moore <paul@paul-moore.com>
This commit is contained in:
parent
d141136f52
commit
0da74120c5
|
@ -441,22 +441,16 @@ static int sel_release_policy(struct inode *inode, struct file *filp)
|
||||||
static ssize_t sel_read_policy(struct file *filp, char __user *buf,
|
static ssize_t sel_read_policy(struct file *filp, char __user *buf,
|
||||||
size_t count, loff_t *ppos)
|
size_t count, loff_t *ppos)
|
||||||
{
|
{
|
||||||
struct selinux_fs_info *fsi = file_inode(filp)->i_sb->s_fs_info;
|
|
||||||
struct policy_load_memory *plm = filp->private_data;
|
struct policy_load_memory *plm = filp->private_data;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
mutex_lock(&fsi->mutex);
|
|
||||||
|
|
||||||
ret = avc_has_perm(&selinux_state,
|
ret = avc_has_perm(&selinux_state,
|
||||||
current_sid(), SECINITSID_SECURITY,
|
current_sid(), SECINITSID_SECURITY,
|
||||||
SECCLASS_SECURITY, SECURITY__READ_POLICY, NULL);
|
SECCLASS_SECURITY, SECURITY__READ_POLICY, NULL);
|
||||||
if (ret)
|
if (ret)
|
||||||
goto out;
|
return ret;
|
||||||
|
|
||||||
ret = simple_read_from_buffer(buf, count, ppos, plm->data, plm->len);
|
return simple_read_from_buffer(buf, count, ppos, plm->data, plm->len);
|
||||||
out:
|
|
||||||
mutex_unlock(&fsi->mutex);
|
|
||||||
return ret;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static vm_fault_t sel_mmap_policy_fault(struct vm_fault *vmf)
|
static vm_fault_t sel_mmap_policy_fault(struct vm_fault *vmf)
|
||||||
|
@ -1188,25 +1182,29 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf,
|
||||||
ret = -EINVAL;
|
ret = -EINVAL;
|
||||||
if (index >= fsi->bool_num || strcmp(name,
|
if (index >= fsi->bool_num || strcmp(name,
|
||||||
fsi->bool_pending_names[index]))
|
fsi->bool_pending_names[index]))
|
||||||
goto out;
|
goto out_unlock;
|
||||||
|
|
||||||
ret = -ENOMEM;
|
ret = -ENOMEM;
|
||||||
page = (char *)get_zeroed_page(GFP_KERNEL);
|
page = (char *)get_zeroed_page(GFP_KERNEL);
|
||||||
if (!page)
|
if (!page)
|
||||||
goto out;
|
goto out_unlock;
|
||||||
|
|
||||||
cur_enforcing = security_get_bool_value(fsi->state, index);
|
cur_enforcing = security_get_bool_value(fsi->state, index);
|
||||||
if (cur_enforcing < 0) {
|
if (cur_enforcing < 0) {
|
||||||
ret = cur_enforcing;
|
ret = cur_enforcing;
|
||||||
goto out;
|
goto out_unlock;
|
||||||
}
|
}
|
||||||
length = scnprintf(page, PAGE_SIZE, "%d %d", cur_enforcing,
|
length = scnprintf(page, PAGE_SIZE, "%d %d", cur_enforcing,
|
||||||
fsi->bool_pending_values[index]);
|
fsi->bool_pending_values[index]);
|
||||||
ret = simple_read_from_buffer(buf, count, ppos, page, length);
|
|
||||||
out:
|
|
||||||
mutex_unlock(&fsi->mutex);
|
mutex_unlock(&fsi->mutex);
|
||||||
|
ret = simple_read_from_buffer(buf, count, ppos, page, length);
|
||||||
|
out_free:
|
||||||
free_page((unsigned long)page);
|
free_page((unsigned long)page);
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
|
out_unlock:
|
||||||
|
mutex_unlock(&fsi->mutex);
|
||||||
|
goto out_free;
|
||||||
}
|
}
|
||||||
|
|
||||||
static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
|
static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
|
||||||
|
@ -1219,6 +1217,17 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
|
||||||
unsigned index = file_inode(filep)->i_ino & SEL_INO_MASK;
|
unsigned index = file_inode(filep)->i_ino & SEL_INO_MASK;
|
||||||
const char *name = filep->f_path.dentry->d_name.name;
|
const char *name = filep->f_path.dentry->d_name.name;
|
||||||
|
|
||||||
|
if (count >= PAGE_SIZE)
|
||||||
|
return -ENOMEM;
|
||||||
|
|
||||||
|
/* No partial writes. */
|
||||||
|
if (*ppos != 0)
|
||||||
|
return -EINVAL;
|
||||||
|
|
||||||
|
page = memdup_user_nul(buf, count);
|
||||||
|
if (IS_ERR(page))
|
||||||
|
return PTR_ERR(page);
|
||||||
|
|
||||||
mutex_lock(&fsi->mutex);
|
mutex_lock(&fsi->mutex);
|
||||||
|
|
||||||
length = avc_has_perm(&selinux_state,
|
length = avc_has_perm(&selinux_state,
|
||||||
|
@ -1233,22 +1242,6 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
|
||||||
fsi->bool_pending_names[index]))
|
fsi->bool_pending_names[index]))
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
length = -ENOMEM;
|
|
||||||
if (count >= PAGE_SIZE)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
/* No partial writes. */
|
|
||||||
length = -EINVAL;
|
|
||||||
if (*ppos != 0)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
page = memdup_user_nul(buf, count);
|
|
||||||
if (IS_ERR(page)) {
|
|
||||||
length = PTR_ERR(page);
|
|
||||||
page = NULL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
length = -EINVAL;
|
length = -EINVAL;
|
||||||
if (sscanf(page, "%d", &new_value) != 1)
|
if (sscanf(page, "%d", &new_value) != 1)
|
||||||
goto out;
|
goto out;
|
||||||
|
@ -1280,6 +1273,17 @@ static ssize_t sel_commit_bools_write(struct file *filep,
|
||||||
ssize_t length;
|
ssize_t length;
|
||||||
int new_value;
|
int new_value;
|
||||||
|
|
||||||
|
if (count >= PAGE_SIZE)
|
||||||
|
return -ENOMEM;
|
||||||
|
|
||||||
|
/* No partial writes. */
|
||||||
|
if (*ppos != 0)
|
||||||
|
return -EINVAL;
|
||||||
|
|
||||||
|
page = memdup_user_nul(buf, count);
|
||||||
|
if (IS_ERR(page))
|
||||||
|
return PTR_ERR(page);
|
||||||
|
|
||||||
mutex_lock(&fsi->mutex);
|
mutex_lock(&fsi->mutex);
|
||||||
|
|
||||||
length = avc_has_perm(&selinux_state,
|
length = avc_has_perm(&selinux_state,
|
||||||
|
@ -1289,22 +1293,6 @@ static ssize_t sel_commit_bools_write(struct file *filep,
|
||||||
if (length)
|
if (length)
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
length = -ENOMEM;
|
|
||||||
if (count >= PAGE_SIZE)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
/* No partial writes. */
|
|
||||||
length = -EINVAL;
|
|
||||||
if (*ppos != 0)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
page = memdup_user_nul(buf, count);
|
|
||||||
if (IS_ERR(page)) {
|
|
||||||
length = PTR_ERR(page);
|
|
||||||
page = NULL;
|
|
||||||
goto out;
|
|
||||||
}
|
|
||||||
|
|
||||||
length = -EINVAL;
|
length = -EINVAL;
|
||||||
if (sscanf(page, "%d", &new_value) != 1)
|
if (sscanf(page, "%d", &new_value) != 1)
|
||||||
goto out;
|
goto out;
|
||||||
|
|
Loading…
Reference in a new issue