Ensure that selinux_log() is thread-safe by guarding the call to the
underlying callback with a mutex.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Not very useful on its own, but will allow to implement a parallel
version of selinux_restorecon() in subsequent patches.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
The 'matches' member of 'struct spec' may be written to by different
threads, so it needs to be accessed using the proper atomic constructs.
Since the actual count of matches doesn't matter and is not used,
convert this field to a bool and just atomically set/read it using GCC
__atomic builtins (which are already being used in another place).
If the compiler lacks support for __atomic builtins (which seem to have
been introduced in GCC 4.1), just fail the compilation. I don't think
it's worth tryin to invent a workaround to support a 15 years old
compiler.
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Glibc 2.34 added an access function attribute to pthread_setspecific(3).
This leads to the following GCC warnings:
In file included from matchpathcon.c:5:
matchpathcon.c: In function ‘matchpathcon_init_prefix’:
selinux_internal.h:38:25: error: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Werror=stringop-overread]
38 | pthread_setspecific(KEY, VALUE); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
matchpathcon.c:359:9: note: in expansion of macro ‘__selinux_setspecific’
359 | __selinux_setspecific(destructor_key, (void *)1);
| ^~~~~~~~~~~~~~~~~~~~~
In file included from selinux_internal.h:2,
from matchpathcon.c:5:
/usr/include/pthread.h:1167:12: note: in a call to function ‘pthread_setspecific’ declared with attribute ‘access (none, 2)’
1167 | extern int pthread_setspecific (pthread_key_t __key,
| ^~~~~~~~~~~~~~~~~~~
The actual value and the validity of the passed pointer is irrelevant,
since it does not gets accessed internally by glibc and
pthread_getspecific(3) is not used.
Use a pointer to a global object to please GCC.
Closes: https://github.com/SELinuxProject/selinux/issues/311
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
An expression of the form "1 << x" is undefined if x == 31 because
the "1" is an int and cannot be left shifted by 31.
Instead, use "UINT32_C(1) << x" which will be an unsigned int of
at least 32 bits.
Signed-off-by: James Carter <jwcart2@gmail.com>
The extra dependency of sefcontext_compile on its object file causes the
compile and link step to be separated.
During the link step the CFLAGS are not passed, which might contain
optimization or sanitizer flags.
Reorder the LDLIBS requirements to avoid the symbol 'pcre_fullinfo'
being unresolvable at link time.
Current behavior:
gcc-11 **custom CFLAGS** -I../include -D_GNU_SOURCE -c -o sefcontext_compile.o sefcontext_compile.c
gcc-11 -L../src sefcontext_compile.o ../src/regex.o -lselinux -lpcre ../src/libselinux.a -lsepol -o sefcontext_compile
Changed:
gcc-11 **custom CFLAGS** -I../include -D_GNU_SOURCE -L../src sefcontext_compile.c -lselinux ../src/libselinux.a -lpcre -lsepol -o sefcontext_compile
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The standard function `strerror(3)` is not thread safe. This does not
only affect the concurrent usage of libselinux itself but also with
other `strerror(3)` linked libraries.
Use the thread safe GNU extension format specifier `%m`[1].
libselinux already uses the GNU extension format specifier `%ms`.
[1]: https://www.gnu.org/software/libc/manual/html_node/Other-Output-Conversions.html
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
On Ubuntu 20.04, when building with clang -Werror -Wextra-semi-stmt
(which is not the default build configuration), the compiler reports:
sha1.c:90:21: error: empty expression statement has no effect;
remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
R0(a,b,c,d,e, 0); R0(e,a,b,c,d, 1); R0(d,e,a,b,c, 2); R0(c,d,e,a,b, 3);
^
In file included from selinux_restorecon.c:39:
./label_file.h:458:15: error: empty expression statement has no
effect; remove unnecessary ';' to silence this warning
[-Werror,-Wextra-semi-stmt]
lineno);
^
Introduce "do { } while (0)" blocks to silence such warnings.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Fix the following build failure with gcc 4.8 which is raised since
version 3.2 and
156dd0de5c
getseuser.c:53:2: error: 'for' loop initial declarations are only allowed in C99 mode
for (int i = 0; i < n; i++)
^
Fixes:
- http://autobuild.buildroot.org/results/37eb0952a763256fbf6ef3c668f6c95fbdf2dd35
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Using mount flag `nosuid` also affects SELinux domain transitions but
this has not been documented well.
Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
Improve formatting of section DESCRIPTION by adding list points.
Mention errno is set on failure.
Mention the returned context might be NULL if SELinux is not enabled.
Align setcon/_raw parameter by adding const.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
In case of a recurring call to `selinux_status_open(3)`, which
previously has been opened in fallback mode, return `1` according to its
documentation.
Fixes: c5a699046f ("libselinux: make selinux_status_open(3) reentrant")
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
Currently `avc_init_internal()`, called by `avc_open(3)` and
`avc_init(3)`, does open the SELinux status page with fallback mode
enabled.
Quote from man:selinux_status_open(3):
In this case, this function tries to open a netlink socket using
.BR avc_netlink_open (3) and overwrite corresponding callbacks
(setenforce and policyload). Thus, we need to pay attention to the
interaction with these interfaces, when fallback mode is enabled.
Calling `selinux_status_open` internally in fallback mode is bad, cause
it overrides callbacks from client applications or the internal
fallback-callbacks get overridden by client applications.
Note that `avc_open(3)` gets called under the hood by
`selinux_check_access(3)` without checking for failure.
Also the status page is available since Linux 2.6.37, so failures of
`selinux_status_open(3)` in non-fallback mode should only be caused by
policies not allowing the client process to open/read/map
the /sys/fs/selinux/status file.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not mmap the status page again if `selinux_status_open(3)` has already
been called with success.
`selinux_status_open(3)` might be called unintentionally multiple times,
e.g. once to manually be able to call `selinux_status_getenforce(3)` and
once indirectly through `selinux_check_access(3)`
(since libselinux 3.2).
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Mention in the manpage of avc_destroy(3) that it does close the SELinux
status page, which might have been opened manually by the client
application.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by clang-tidy.
libselinux/src/label_file.c:374:4: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
else
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by clang-tidy.
libselinux/src/avc_sidtab.h:32:6: warning: function 'sidtab_sid_stats' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]
void sidtab_sid_stats(struct sidtab *s, char *buf, int buflen) ;
^
libselinux/src/avc_sidtab.c:103:6: note: the definition seen here
void sidtab_sid_stats(struct sidtab *h, char *buf, int buflen)
^
libselinux/src/avc_sidtab.h:32:6: note: differing parameters are named here: ('s'), in definition: ('h')
void sidtab_sid_stats(struct sidtab *s, char *buf, int buflen) ;
^ ~
h
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Open the file stream with the `e` flag, so that the underlying file
descriptor gets closed on an exec in a potential sibling thread.
Also drop the flag `b`, since it is ignored on POSIX systems.
Found by clang-tidy.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
In case `realloc()` fails and returns NULL, free the passed array,
instead of just setting the size helper variables to 0.
Also free the string contents in `free_array_elts()` of the array
`con_array`, instead of just the array of pointers.
Found by cppcheck.
src/matchpathcon.c:86:4: error: Common realloc mistake: 'con_array' nulled but not freed upon failure [memleakOnRealloc]
con_array = (char **)realloc(con_array, sizeof(char*) *
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
If any of the build flags `BUILD_HOST` or `ANDROID` is set and the
caller did not pass an option of type `SELABEL_OPT_PATH`, the variable
`path` might be not set.
Add a check to avoid calling `strdup()` with a NULL pointer.
Found by cppcheck.
src/label_file.c:759:26: warning: Possible null pointer dereference: path [nullPointer]
rec->spec_file = strdup(path);
^
src/label_file.c:713:21: note: Assignment 'path=NULL', assigned value is 0
const char *path = NULL;
^
src/label_file.c:759:26: note: Null pointer dereference
rec->spec_file = strdup(path);
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by Infer.
selinux_config.c:181: error: Resource Leak
resource of type `_IO_FILE` acquired by call to `fopen()` at line 165, column 7 is not released after line 181, column 6.
179. type = strdup(buf_p + sizeof(SELINUXTYPETAG) - 1);
180. if (!type)
181. return;
^
182. end = type + strlen(type) - 1;
183. while ((end > type) &&
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Found by Infer.
matchmediacon.c:25: error: Resource Leak
resource of type `_IO_FILE` acquired to `return` by call to `fopen()` at line 21, column 16 is not released after line 25, column 4.
23. while (!feof_unlocked(infile)) {
24. if (!fgets_unlocked(current_line, sizeof(current_line), infile)) {
25. return -1;
^
26. }
27. if (current_line[strlen(current_line) - 1])
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
GCC 11 complains:
In file included from label_file.c:24:
In function ‘store_stem’,
inlined from ‘load_mmap’ at label_file.c:277:12,
inlined from ‘process_file’ at label_file.c:551:5:
label_file.h:289:25: error: ‘free’ called on pointer ‘*mmap_area.next_addr’ with nonzero offset 4 [-Werror=free-nonheap-object]
289 | free(buf);
| ^~~~~~~~~
Free the pointer on failure at the caller instead of inside `store_stem()`.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not leak memory if program arguments get specified more than once.
Found by clang-anlyzer.
getdefaultcon.c:52:3: warning: Potential leak of memory pointed to by 'level' [unix.Malloc]
fprintf(stderr,
^~~~~~~~~~~~~~~
getdefaultcon.c:52:3: warning: Potential leak of memory pointed to by 'role' [unix.Malloc]
fprintf(stderr,
^~~~~~~~~~~~~~~
getdefaultcon.c:52:3: warning: Potential leak of memory pointed to by 'service' [unix.Malloc]
fprintf(stderr,
^~~~~~~~~~~~~~~
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `rc` is always unconditionally assigned by the next call of
`setexeccon()` and never read in between.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `lineno` is only used in the preceding loop and it always
set prior that to 0.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `lineno` is only used in the preceding loop and is always
set prior that to 0.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `i` is not used inside this loop, and it later
unconditionally set to 0.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not leak memory if the program argument `l` got passed more than
once.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Free all memory from `selabel_get_digests_all_partial_matches()` in case
of success and failure.
Found by clang-analyzer.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The variable `dir_xattr_list` is only used inside `selinux_restorecon.c`.
selinux_restorecon.c:65:19: warning: no previous extern declaration for non-static variable 'dir_xattr_list' [-Wmissing-variable-declarations]
struct dir_xattr *dir_xattr_list;
^
selinux_restorecon.c:65:1: note: declare 'static' if the variable is not intended to be used outside of this translation unit
struct dir_xattr *dir_xattr_list;
^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
The format width specifier `L` is only standardized for floating point
types. Use `ll` for fixed-width data types.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Mark the argument `Buffer` of `Sha1Update()` const, since it is not
modified.
sha1.c: In function ‘Sha1Finalise’:
sha1.c:208:25: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
208 | Sha1Update(Context, (uint8_t*)"\x80", 1);
| ^
sha1.c:211:29: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
211 | Sha1Update(Context, (uint8_t*)"\0", 1);
| ^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
As the const qualifier is discarded in label_common(), do not return a
const qualified pointer pointer from the local function `lookup_all()`.
label_file.c: In function ‘lookup_common’:
label_file.c:994:24: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
994 | struct spec *result = (struct spec*)matches[0];
| ^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Do not discard the const qualifier of the function argument, and drop
the redundant local variable `keyp`.
avc_sidtab.c: In function ‘sidtab_hash’:
avc_sidtab.c:23:9: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
23 | keyp = (char *)key;
| ^
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When building libselinux on Fedora 33 with gcc 10.3.1, the compiler
reports:
label_file.c: In function ‘lookup_all.isra’:
label_file.c:940:4: error: ‘strncpy’ specified bound depends on the
length of the source argument [-Werror=stringop-overflow=]
940 | strncpy(clean_key, key, len - 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
label_file.c:927:8: note: length computed here
927 | len = strlen(key);
| ^~~~~~~~~~~
cc1: all warnings being treated as errors
As clean_key is the result of malloc(len), there is no issue here. But
using strncpy can be considered as strange, because the size of the
string is already known and the NUL terminator is always added later, in
function ‘lookup_all.isra.
Replace strncpy with memcpy to silence this gcc false-positive warning.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Acked-by: Petr Lautrbach <plautrba@redhat.com>
`selinux_check_passwd_access_internal()`, and thereby
`checkPasswdAccess(3)` and `selinux_check_passwd_access(3)`, does not
respect the policy defined setting of `deny_unknown`, like
`selinux_check_access(3)` does.
This means in case the security class `passwd` is not defined, success
is returned instead of failure, i.e. permission denied.
Most policies should define the `passwd` class and the two affected
public functions are marked deprecated.
Align the behavior with `selinux_check_access(3)` and respect
the deny_unknown setting in case the security class is not defined.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
When running "make install-pywrap", make displays:
make[1]: Entering directory '/root/selinux/libselinux'
make -C src install-pywrap install-pywrap
make[2]: Entering directory '/root/selinux/libselinux/src'
The duplicated "install-pywrap" is not expected. Remove it from the
Makefile.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
