Skip to content

Commit d313f94

Browse files
stephensmalleyJames Morris
stephensmalley
authored and
James Morris
committed
SELinux: detect dead booleans
Instead of using f_op to detect dead booleans, check the inode index against the number of booleans and check the dentry name against the boolean name for that index on reads and writes. This prevents incorrect use of a boolean file opened prior to a policy reload while allowing valid use of it as long as it still corresponds to the same boolean in the policy. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: James Morris <jmorris@namei.org>
1 parent 0955dc0 commit d313f94

File tree

1 file changed

+30
-13
lines changed

1 file changed

+30
-13
lines changed

‎security/selinux/selinuxfs.c

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ static DEFINE_MUTEX(sel_mutex);
6565
/* global data for booleans */
6666
static struct dentry *bool_dir = NULL;
6767
static int bool_num = 0;
68+
static char **bool_pending_names;
6869
static int *bool_pending_values = NULL;
6970

7071
/* global data for classes */
@@ -832,11 +833,16 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf,
832833
ssize_t length;
833834
ssize_t ret;
834835
int cur_enforcing;
835-
struct inode *inode;
836+
struct inode *inode = filep->f_path.dentry->d_inode;
837+
unsigned index = inode->i_ino & SEL_INO_MASK;
838+
const char *name = filep->f_path.dentry->d_name.name;
836839

837840
mutex_lock(&sel_mutex);
838841

839-
ret = -EFAULT;
842+
if (index >= bool_num || strcmp(name, bool_pending_names[index])) {
843+
ret = -EINVAL;
844+
goto out;
845+
}
840846

841847
if (count > PAGE_SIZE) {
842848
ret = -EINVAL;
@@ -847,15 +853,13 @@ static ssize_t sel_read_bool(struct file *filep, char __user *buf,
847853
goto out;
848854
}
849855

850-
inode = filep->f_path.dentry->d_inode;
851-
cur_enforcing = security_get_bool_value(inode->i_ino&SEL_INO_MASK);
856+
cur_enforcing = security_get_bool_value(index);
852857
if (cur_enforcing < 0) {
853858
ret = cur_enforcing;
854859
goto out;
855860
}
856-
857861
length = scnprintf(page, PAGE_SIZE, "%d %d", cur_enforcing,
858-
bool_pending_values[inode->i_ino&SEL_INO_MASK]);
862+
bool_pending_values[index]);
859863
ret = simple_read_from_buffer(buf, count, ppos, page, length);
860864
out:
861865
mutex_unlock(&sel_mutex);
@@ -868,22 +872,31 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
868872
size_t count, loff_t *ppos)
869873
{
870874
char *page = NULL;
871-
ssize_t length = -EFAULT;
875+
ssize_t length;
872876
int new_value;
873-
struct inode *inode;
877+
struct inode *inode = filep->f_path.dentry->d_inode;
878+
unsigned index = inode->i_ino & SEL_INO_MASK;
879+
const char *name = filep->f_path.dentry->d_name.name;
874880

875881
mutex_lock(&sel_mutex);
876882

877883
length = task_has_security(current, SECURITY__SETBOOL);
878884
if (length)
879885
goto out;
880886

887+
if (index >= bool_num || strcmp(name, bool_pending_names[index])) {
888+
length = -EINVAL;
889+
goto out;
890+
}
891+
881892
if (count >= PAGE_SIZE) {
882893
length = -ENOMEM;
883894
goto out;
884895
}
896+
885897
if (*ppos != 0) {
886898
/* No partial writes. */
899+
length = -EINVAL;
887900
goto out;
888901
}
889902
page = (char*)get_zeroed_page(GFP_KERNEL);
@@ -892,6 +905,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
892905
goto out;
893906
}
894907

908+
length = -EFAULT;
895909
if (copy_from_user(page, buf, count))
896910
goto out;
897911

@@ -902,8 +916,7 @@ static ssize_t sel_write_bool(struct file *filep, const char __user *buf,
902916
if (new_value)
903917
new_value = 1;
904918

905-
inode = filep->f_path.dentry->d_inode;
906-
bool_pending_values[inode->i_ino&SEL_INO_MASK] = new_value;
919+
bool_pending_values[index] = new_value;
907920
length = count;
908921

909922
out:
@@ -923,7 +936,7 @@ static ssize_t sel_commit_bools_write(struct file *filep,
923936
size_t count, loff_t *ppos)
924937
{
925938
char *page = NULL;
926-
ssize_t length = -EFAULT;
939+
ssize_t length;
927940
int new_value;
928941

929942
mutex_lock(&sel_mutex);
@@ -946,6 +959,7 @@ static ssize_t sel_commit_bools_write(struct file *filep,
946959
goto out;
947960
}
948961

962+
length = -EFAULT;
949963
if (copy_from_user(page, buf, count))
950964
goto out;
951965

@@ -1010,7 +1024,9 @@ static int sel_make_bools(void)
10101024
u32 sid;
10111025

10121026
/* remove any existing files */
1027+
kfree(bool_pending_names);
10131028
kfree(bool_pending_values);
1029+
bool_pending_names = NULL;
10141030
bool_pending_values = NULL;
10151031

10161032
sel_remove_entries(dir);
@@ -1052,16 +1068,17 @@ static int sel_make_bools(void)
10521068
d_add(dentry, inode);
10531069
}
10541070
bool_num = num;
1071+
bool_pending_names = names;
10551072
bool_pending_values = values;
10561073
out:
10571074
free_page((unsigned long)page);
1075+
return ret;
1076+
err:
10581077
if (names) {
10591078
for (i = 0; i < num; i++)
10601079
kfree(names[i]);
10611080
kfree(names);
10621081
}
1063-
return ret;
1064-
err:
10651082
kfree(values);
10661083
sel_remove_entries(dir);
10671084
ret = -ENOMEM;

0 commit comments

Comments
 (0)