Rebase on db0f2f382e31 at SELinuxProject

- Optimize storage of filename transitions
- Fix signed overflow caused by using (1 << 31) - 1
This commit is contained in:
Petr Lautrbach 2020-11-01 14:36:16 +01:00
parent 611818c50a
commit 497bda00f7
3 changed files with 223 additions and 2 deletions

View File

@ -0,0 +1,129 @@
From 42ae834a7428c57f7b2a9f448adf4cf991fa3487 Mon Sep 17 00:00:00 2001
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: Fri, 31 Jul 2020 13:10:34 +0200
Subject: [PATCH] libsepol,checkpolicy: optimize storage of filename
transitions
In preparation to support a new policy format with a more optimal
representation of filename transition rules, this patch applies an
equivalent change from kernel commit c3a276111ea2 ("selinux: optimize
storage of filename transitions").
See the kernel commit's description [1] for the rationale behind this
representation. This change doesn't bring any measurable difference of
policy build performance (semodule -B) on Fedora.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git/commit/?id=c3a276111ea2572399281988b3129683e2a6b60b
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
checkpolicy/policy_define.c | 49 ++++++++++---------------------------
checkpolicy/test/dispol.c | 20 ++++++++++-----
2 files changed, 27 insertions(+), 42 deletions(-)
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index c6733fa469c5..395f62284e3c 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -3303,8 +3303,6 @@ int define_filename_trans(void)
ebitmap_t e_stypes, e_ttypes;
ebitmap_t e_tclasses;
ebitmap_node_t *snode, *tnode, *cnode;
- filename_trans_t *ft;
- filename_trans_datum_t *ftdatum;
filename_trans_rule_t *ftr;
type_datum_t *typdatum;
uint32_t otype;
@@ -3388,40 +3386,19 @@ int define_filename_trans(void)
ebitmap_for_each_positive_bit(&e_tclasses, cnode, c) {
ebitmap_for_each_positive_bit(&e_stypes, snode, s) {
ebitmap_for_each_positive_bit(&e_ttypes, tnode, t) {
- ft = calloc(1, sizeof(*ft));
- if (!ft) {
- yyerror("out of memory");
- goto bad;
- }
- ft->stype = s+1;
- ft->ttype = t+1;
- ft->tclass = c+1;
- ft->name = strdup(name);
- if (!ft->name) {
- yyerror("out of memory");
- goto bad;
- }
-
- ftdatum = hashtab_search(policydbp->filename_trans,
- (hashtab_key_t)ft);
- if (ftdatum) {
- yyerror2("duplicate filename transition for: filename_trans %s %s %s:%s",
- name,
- policydbp->p_type_val_to_name[s],
- policydbp->p_type_val_to_name[t],
- policydbp->p_class_val_to_name[c]);
- goto bad;
- }
-
- ftdatum = calloc(1, sizeof(*ftdatum));
- if (!ftdatum) {
- yyerror("out of memory");
- goto bad;
- }
- rc = hashtab_insert(policydbp->filename_trans,
- (hashtab_key_t)ft,
- ftdatum);
- if (rc) {
+ rc = policydb_filetrans_insert(
+ policydbp, s+1, t+1, c+1, name,
+ NULL, otype, NULL
+ );
+ if (rc != SEPOL_OK) {
+ if (rc == SEPOL_EEXIST) {
+ yyerror2("duplicate filename transition for: filename_trans %s %s %s:%s",
+ name,
+ policydbp->p_type_val_to_name[s],
+ policydbp->p_type_val_to_name[t],
+ policydbp->p_class_val_to_name[c]);
+ goto bad;
+ }
yyerror("out of memory");
goto bad;
}
diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
index d72d9fb331cf..8785b7252824 100644
--- a/checkpolicy/test/dispol.c
+++ b/checkpolicy/test/dispol.c
@@ -335,17 +335,25 @@ static int filenametr_display(hashtab_key_t key,
hashtab_datum_t datum,
void *ptr)
{
- struct filename_trans *ft = (struct filename_trans *)key;
+ struct filename_trans_key *ft = (struct filename_trans_key *)key;
struct filename_trans_datum *ftdatum = datum;
struct filenametr_display_args *args = ptr;
policydb_t *p = args->p;
FILE *fp = args->fp;
+ ebitmap_node_t *node;
+ uint32_t bit;
+
+ do {
+ ebitmap_for_each_positive_bit(&ftdatum->stypes, node, bit) {
+ display_id(p, fp, SYM_TYPES, bit, "");
+ display_id(p, fp, SYM_TYPES, ft->ttype - 1, "");
+ display_id(p, fp, SYM_CLASSES, ft->tclass - 1, ":");
+ display_id(p, fp, SYM_TYPES, ftdatum->otype - 1, "");
+ fprintf(fp, " %s\n", ft->name);
+ }
+ ftdatum = ftdatum->next;
+ } while (ftdatum);
- display_id(p, fp, SYM_TYPES, ft->stype - 1, "");
- display_id(p, fp, SYM_TYPES, ft->ttype - 1, "");
- display_id(p, fp, SYM_CLASSES, ft->tclass - 1, ":");
- display_id(p, fp, SYM_TYPES, ftdatum->otype - 1, "");
- fprintf(fp, " %s\n", ft->name);
return 0;
}
--
2.29.0

View File

@ -0,0 +1,90 @@
From 521e6a2f478a4c7a7c198c017d4d12e8667d89e7 Mon Sep 17 00:00:00 2001
From: Nicolas Iooss <nicolas.iooss@m4x.org>
Date: Sat, 3 Oct 2020 15:19:08 +0200
Subject: [PATCH] libsepol/cil: fix signed overflow caused by using (1 << 31) -
1
When compiling SELinux userspace tools with -ftrapv (this option
generates traps for signed overflow on addition, subtraction,
multiplication operations, instead of silently wrapping around),
semodule crashes when running the tests from
scripts/ci/fedora-test-runner.sh in a Fedora 32 virtual machine:
[root@localhost selinux-testsuite]# make test
make -C policy load
make[1]: Entering directory '/root/selinux-testsuite/policy'
# Test for "expand-check = 0" in /etc/selinux/semanage.conf
# General policy build
make[2]: Entering directory '/root/selinux-testsuite/policy/test_policy'
Compiling targeted test_policy module
Creating targeted test_policy.pp policy package
rm tmp/test_policy.mod.fc
make[2]: Leaving directory '/root/selinux-testsuite/policy/test_policy'
# General policy load
domain_fd_use --> off
/usr/sbin/semodule -i test_policy/test_policy.pp test_mlsconstrain.cil test_overlay_defaultrange.cil test_add_levels.cil test_glblub.cil
make[1]: *** [Makefile:174: load] Aborted (core dumped)
Using "coredumpctl gdb" leads to the following strack trace:
(gdb) bt
#0 0x00007f608fe4fa25 in raise () from /lib64/libc.so.6
#1 0x00007f608fe38895 in abort () from /lib64/libc.so.6
#2 0x00007f6090028aca in __addvsi3.cold () from /lib64/libsepol.so.1
#3 0x00007f6090096f59 in __avrule_xperm_setrangebits (low=30, high=30, xperms=0x8b9eea0)
at ../cil/src/cil_binary.c:1551
#4 0x00007f60900970dd in __cil_permx_bitmap_to_sepol_xperms_list (xperms=0xb650a30, xperms_list=0x7ffce2653b18)
at ../cil/src/cil_binary.c:1596
#5 0x00007f6090097286 in __cil_avrulex_ioctl_to_policydb (k=0xb8ec200 "@\023\214\022\006", datum=0xb650a30,
args=0x239a640) at ../cil/src/cil_binary.c:1649
#6 0x00007f609003f1e5 in hashtab_map (h=0x41f8710, apply=0x7f60900971da <__cil_avrulex_ioctl_to_policydb>,
args=0x239a640) at hashtab.c:234
#7 0x00007f609009ea19 in cil_binary_create_allocated_pdb (db=0x2394f10, policydb=0x239a640)
at ../cil/src/cil_binary.c:4969
#8 0x00007f609009d19d in cil_binary_create (db=0x2394f10, policydb=0x7ffce2653d30) at ../cil/src/cil_binary.c:4329
#9 0x00007f609008ec23 in cil_build_policydb_create_pdb (db=0x2394f10, sepol_db=0x7ffce2653d30)
at ../cil/src/cil.c:631
#10 0x00007f608fff4bf3 in semanage_direct_commit () from /lib64/libsemanage.so.1
#11 0x00007f608fff9fae in semanage_commit () from /lib64/libsemanage.so.1
#12 0x0000000000403e2b in main (argc=7, argv=0x7ffce2655058) at semodule.c:753
(gdb) f 3
#3 0x00007f6090096f59 in __avrule_xperm_setrangebits (low=30, high=30, xperms=0x8b9eea0)
at ../cil/src/cil_binary.c:1551
1551 xperms->perms[i] |= XPERM_SETBITS(h) - XPERM_SETBITS(low);
A signed integer overflow therefore occurs in XPERM_SETBITS(h):
#define XPERM_SETBITS(x) ((1 << (x & 0x1f)) - 1)
This macro is expanded with h=31, so "(1 << 31) - 1" is computed:
* (1 << 31) = -0x80000000 is the lowest signed 32-bit integer value
* (1 << 31) - 1 overflows the capacity of a signed 32-bit integer and
results in 0x7fffffff (which is unsigned)
Using unsigned integers (with "1U") fixes the crash, as
(1U << 31) = 0x80000000U has no overflowing issues.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
---
checkpolicy/policy_define.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 395f62284e3c..bf6c3e68bef3 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -2147,7 +2147,7 @@ out:
/* index of the u32 containing the permission */
#define XPERM_IDX(x) (x >> 5)
/* set bits 0 through x-1 within the u32 */
-#define XPERM_SETBITS(x) ((1 << (x & 0x1f)) - 1)
+#define XPERM_SETBITS(x) ((1U << (x & 0x1f)) - 1)
/* low value for this u32 */
#define XPERM_LOW(x) (x << 5)
/* high value for this u32 */
--
2.29.0

View File

@ -1,5 +1,5 @@
%define libselinuxver 3.1 %define libselinuxver 3.1-4
%define libsepolver 3.1 %define libsepolver 3.1-4
Summary: SELinux policy compiler Summary: SELinux policy compiler
Name: checkpolicy Name: checkpolicy
@ -12,6 +12,8 @@ Source0: https://github.com/SELinuxProject/selinux/releases/download/20200710/ch
# $ git format-patch -N checkpolicy-3.1 -- checkpolicy # $ git format-patch -N checkpolicy-3.1 -- checkpolicy
# $ i=1; for j in 00*patch; do printf "Patch%04d: %s\n" $i $j; i=$((i+1));done # $ i=1; for j in 00*patch; do printf "Patch%04d: %s\n" $i $j; i=$((i+1));done
# Patch list start # Patch list start
Patch0001: 0001-libsepol-checkpolicy-optimize-storage-of-filename-tr.patch
Patch0002: 0002-libsepol-cil-fix-signed-overflow-caused-by-using-1-3.patch
# Patch list end # Patch list end
BuildRequires: gcc BuildRequires: gcc
BuildRequires: byacc bison flex flex-static libsepol-static >= %{libsepolver} libselinux-devel >= %{libselinuxver} BuildRequires: byacc bison flex flex-static libsepol-static >= %{libsepolver} libselinux-devel >= %{libselinuxver}