selinux: fix cond_list corruption when changing booleans
commit d8f5f0ea5b86300390b026b6c6e7836b7150814a upstream.
Currently, duplicate_policydb_cond_list() first copies the whole
conditional avtab and then tries to link to the correct entries in
cond_dup_av_list() using avtab_search(). However, since the conditional
avtab may contain multiple entries with the same key, this approach
often fails to find the right entry, potentially leading to wrong rules
being activated/deactivated when booleans are changed.
To fix this, instead start with an empty conditional avtab and add the
individual entries one-by-one while building the new av_lists. This
approach leads to the correct result, since each entry is present in the
av_lists exactly once.
The issue can be reproduced with Fedora policy as follows:
# sesearch -s ftpd_t -t public_content_rw_t -c dir -p create -A
allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
allow ftpd_t public_content_rw_t:dir { add_name create link remove_name rename reparent rmdir setattr unlink watch watch_reads write }; [ ftpd_anon_write ]:True
# setsebool ftpd_anon_write=off ftpd_connect_all_unreserved=off ftpd_connect_db=off ftpd_full_access=off
On fixed kernels, the sesearch output is the same after the setsebool
command:
# sesearch -s ftpd_t -t public_content_rw_t -c dir -p create -A
allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
allow ftpd_t public_content_rw_t:dir { add_name create link remove_name rename reparent rmdir setattr unlink watch watch_reads write }; [ ftpd_anon_write ]:True
While on the broken kernels, it will be different:
# sesearch -s ftpd_t -t public_content_rw_t -c dir -p create -A
allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
allow ftpd_t non_security_file_type:dir { add_name create getattr ioctl link lock open read remove_name rename reparent rmdir search setattr unlink watch watch_reads write }; [ ftpd_full_access ]:True
While there, also simplify the computation of nslots. This changes the
nslots values for nrules 2 or 3 to just two slots instead of 4, which
makes the sequence more consistent.
Cc: stable@vger.kernel.org
Fixes: c7c556f1e8
("selinux: refactor changing booleans")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:

committed by
Greg Kroah-Hartman

parent
4f29b08e23
commit
fd75d73aa2
@@ -605,7 +605,6 @@ static int cond_dup_av_list(struct cond_av_list *new,
|
||||
struct cond_av_list *orig,
|
||||
struct avtab *avtab)
|
||||
{
|
||||
struct avtab_node *avnode;
|
||||
u32 i;
|
||||
|
||||
memset(new, 0, sizeof(*new));
|
||||
@@ -615,10 +614,11 @@ static int cond_dup_av_list(struct cond_av_list *new,
|
||||
return -ENOMEM;
|
||||
|
||||
for (i = 0; i < orig->len; i++) {
|
||||
avnode = avtab_search_node(avtab, &orig->nodes[i]->key);
|
||||
if (WARN_ON(!avnode))
|
||||
return -EINVAL;
|
||||
new->nodes[i] = avnode;
|
||||
new->nodes[i] = avtab_insert_nonunique(avtab,
|
||||
&orig->nodes[i]->key,
|
||||
&orig->nodes[i]->datum);
|
||||
if (!new->nodes[i])
|
||||
return -ENOMEM;
|
||||
new->len++;
|
||||
}
|
||||
|
||||
@@ -630,7 +630,7 @@ static int duplicate_policydb_cond_list(struct policydb *newp,
|
||||
{
|
||||
int rc, i, j;
|
||||
|
||||
rc = avtab_duplicate(&newp->te_cond_avtab, &origp->te_cond_avtab);
|
||||
rc = avtab_alloc_dup(&newp->te_cond_avtab, &origp->te_cond_avtab);
|
||||
if (rc)
|
||||
return rc;
|
||||
|
||||
|
Reference in New Issue
Block a user