Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Libsepol performace patches #60

Closed
wants to merge 39 commits into from

Conversation

WOnder93
Copy link
Member

@WOnder93 WOnder93 commented Jul 3, 2020

bachradsusi and others added 30 commits December 6, 2019 06:58
…t null.

Return errno EINVAL, to prevent segfault.

Rejected by upstream https://marc.info/?l=selinux&m=145036088424584&w=2

FIXME: use __attribute__(nonnull (arg-index, ...))
Signed-off-by: Miroslav Grepl <mgrepl@redhat.com>
… no longer hardcoded and it creates only index.html and html man pages in the directory for the system release.
…_is_entrypoint(f)

- use direct queries
- load exec_types and entry_types only once
When policycoreutils was split into policycoreutils/ python/ gui/ and sandbox/
sub-directories, po/ translation files stayed in policycoreutils/.

This commit split original policycoreutils/po directory into
policycoreutils/po
python/po
gui/po
sandbox/po

See fedora-selinux#43
Usage:
zanata-cli -e pull --pull-type trans
zanata-cli -e push --push-type source

fedora-selinux#43
The "-q" switch is becoming obsolete (completely unused in fedora) and
debug output ("-d" switch) makes sense in any scenario. Therefore both
options can be specified at once.

Resolves: rhbz#1271327
Currently only reserved_port_t, port_t and hi_reserved_port_t are
handled as special when making a ports-dictionary.  However, as fas as
corenetwork.te.in of serefpolicy, unreserved_port_t and
ephemeral_port_t should be handled in the same way, too.

(Details) I found the need of this change when I was using
selinux-polgengui.  Though tcp port 12345, which my application may
use, was given to the gui, selinux-polgengui generates expected te
file and sh file which didn't utilize the tcp port.

selinux-polgengui checks whether a port given via gui is already typed
or not.

If it is already typed, selinux-polgengui generates a te file having
rules to allow the application to use the port. (A)

If not, it seems for me that selinux-polgengui is designed to generate
a te file having rules to allow the application to own(?) the port;
and a sh file having a command line to assign the application own type
to the port. (B)

As we can see the output of `semanage port -l' some of ports for
specified purpose have types already.  The important point is that the
rest of ports also have types already:

    hi_reserved_port_t tcp 512-1023
    hi_reserved_port_t udp 512-1023
    unreserved_port_t tcp 1024-32767, 61001-65535
    unreserved_port_t udp 1024-32767, 61001-65535
    ephemeral_port_t tcp 32768-61000
    ephemeral_port_t udp 32768-61000

As my patch shows, the original selinux-polgengui ignored
hi_reserved_port_t; though hi_reserved_port_t is assigned,
selinux-polgengui considered ports 512-1023 are not used. As the
result selinux-polgengui generates file sets of (B).

For the purpose of selinux-polgengui, I think unreserved_port_t and
ephemeral_port_t are treated as the same as hi_reserved_port_t.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>

Fedora only patch:
https://lore.kernel.org/selinux/20150610.190635.1866127952891120915.yamato@redhat.com/
ipaddress module was added in python 3.3 and this allows us to drop python3-IPy
Fixes:
$ python3
> import selinux
> selinux.selinux_raw_context_to_color("xyz_u:xyz_r:xyz_t:")

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
OSError: [Errno 0] Error

:: [ 10:25:45 ] :: [  BEGIN   ] :: Running 'service mcstransd status'
Redirecting to /bin/systemctl status mcstransd.service
● mcstrans.service - Translates SELinux MCS/MLS labels to human readable form
   Loaded: loaded (/usr/lib/systemd/system/mcstrans.service; disabled; vendor preset: disabled)
   Active: failed (Result: core-dump) since Fri 2019-04-12 10:25:44 EDT; 1s ago
  Process: 16681 ExecStart=/sbin/mcstransd -f (code=dumped, signal=SEGV)
 Main PID: 16681 (code=dumped, signal=SEGV)

systemd[1]: mcstrans.service: Main process exited, code=dumped, status=11/SEGV
systemd[1]: mcstrans.service: Failed with result 'core-dump'.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
…dling

When copying an avrule with extended permissions (permx) in
cil_copy_avrule(), the check for a named permx checks the new permx
instead of the old one, so the check will always fail. This leads to a
segfault when trying to copy a named permx because there will be an
attempt to copy the nonexistent permx struct instead of the name of
the named permx.

Check whether the original is a named permx instead of the new one.

Signed-off-by: James Carter <jwcart2@tycho.nsa.gov>
GCC 10 comes with -fno-common enabled by default - fix the CIL_KEY_*
global variables to be defined only once in cil.c and declared in the
header file correctly with the 'extern' keyword, so that other units
including the file don't generate duplicate definitions.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Commit 4459d63 ("libsepol: Remove cil_mem_error_handler() function
pointer") replaced cil_mem_error_handler usage with inline contents of
the default handler. However, it left over the header declaration and
two callers. Convert these as well and remove the header declaration.

This also fixes a build failure with -fno-common.

Fixes: 4459d63 ("libsepol: Remove cil_mem_error_handler() function pointer")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Rename flush_class_cache() to selinux_flush_class_cache(), export it
for direct use by userspace policy enforcers, and call it on all policy
load notifications rather than only when using selinux_check_access().
This ensures that policy reloads that change a userspace class or
permission value will be reflected by subsequent string_to_security_class()
or string_to_av_perm() calls.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
get_ordered_context_list() code used to ask the kernel to compute the complete
set of reachable contexts using /sys/fs/selinux/user aka
security_compute_user(). This set can be so huge so that it doesn't fit into a
kernel page and security_compute_user() fails. Even if it doesn't fail,
get_ordered_context_list() throws away the vast majority of the returned
contexts because they don't match anything in
/etc/selinux/targeted/contexts/default_contexts or
/etc/selinux/targeted/contexts/users/

get_ordered_context_list() is rewritten to compute set of contexts based on
/etc/selinux/targeted/contexts/users/ and
/etc/selinux/targeted/contexts/default_contexts files and to return only valid
contexts, using security_check_context(), from this set.

Fixes: SELinuxProject/selinux#28

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
stephensmalley and others added 9 commits March 5, 2020 14:36
commit 1f89c4e ("libselinux: Eliminate
use of security_compute_user()") eliminated the use of
security_compute_user() by get_ordered_context_list().  Deprecate
all use of security_compute_user() by updating the headers and man
pages and logging a warning message on any calls to it.  Remove
the example utility that called the interface. While here, also
fix the documentation of correct usage of the user argument to these
interfaces.

Fixes: SELinuxProject/selinux#70
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
The filename_- and range_trans_table ancillary hash tables in
cil_binary.c just duplicate the final policydb content and can be simply
removed.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
According to profiling of semodule -BN, ebitmap_cardinality() is called
quite often and contributes a lot to the total runtime. Cache its result
in the ebitmap struct to reduce this overhead. The cached value is
invalidated on most modifying operations, but ebitmap_cardinality() is
usually called once the ebitmap doesn't change any more.

After this patch, the time to do 'semodule -BN' on Fedora Rawhide has
decreased from ~10.9s to ~8.9s (2s saved).

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
[sds@tycho.nsa.gov: correct times per follow-up on list]
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Detect when the hashtab's load factor gets too high and try to grow it
and rehash it in such case. If the reallocation fails, just keep the
hashtab at its current size, since this is not a fatal error (it will
just be slower).

This speeds up semodule -BN on Fedora from ~8.9s to ~7.2s (1.7 seconds
saved).

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
As ebitmap_get_bit() complexity is linear in the size of the bitmap, the
complexity of ebitmap_cardinality() is quadratic. This can be optimized
by browsing the nodes of the bitmap directly in ebitmap_cardinality().

While at it, use built-in function __builtin_popcountll() to count the
ones in the 64-bit value n->map for each bitmap node. This seems better
suited than "count++". This seems to work on gcc and clang on x86,
x86_64, ARM and ARM64 but if it causes compatibility issues with some
compilers or architectures (or with older versions of gcc or clang),
the use of __builtin_popcountll() can be replaced by a C implementation
of a popcount algorithm.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This reverts commit 542e878.

After 6968ea9 ("libsepol: make ebitmap_cardinality() of linear
complexity"), the caching only saves ~0.06 % of total semodule -BN
running time (on x86_64 without using the POPCNT instruction), so it's
no longer worth the added complexity.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
I copy-pasted it from a different part of the code, which had to deal
with policydb that isn't final yet. Since we only deal with the final
kernel policy here, we can skip the check for the type datum being NULL.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Only attributes can be a superset of another attribute, so we can skip
non-attributes right away.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
The iteration over the set ebitmap bits is not implemented very
efficiently in libsepol. It is slowing down the policy optimization
quite significantly, so convert the type_map from an array of ebitmaps
to an array of simple ordered vectors, which can be traveresed more
easily. The worse space efficiency of the vectors is less important than
the speed in this case.

After this change the duration of semodule -BN decreased from 6.4s to
5.5s on Fedora Rawhide x86_64 (and from 6.1s to 5.6s with the unconfined
module disabled).

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
@bachradsusi
Copy link
Member

Fixed by 7df27b7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants