Proposing SELinux support in strace
Renaud Métrich
rmetrich at redhat.com
Mon Mar 15 17:09:34 UTC 2021
Hi Dmitry, thanks for the review.
Please see inline. Attached is the newest patch with your recommendations.
Thanks,
Renaud.
On 3/15/21 2:10 AM, Dmitry V. Levin wrote:
>
>> @@ -126,6 +128,18 @@ case "$arch" in
>> else
>> st_cv_mpers=no
>> fi])
>> + AC_CACHE_CHECK([whether selinux runtime works with mpers_name personality],
>> + [st_cv_selinux_runtime],
>> + [
>> + saved_LIBS="$LIBS"
>> + LIBS="-lselinux $LIBS"
>> + AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include <selinux/selinux.h>]],
>> + [[return 0]])],
>> + [st_cv_selinux_runtime=yes],
>> + [st_cv_selinux_runtime=no],
>> + [st_cv_selinux_runtime=no])
>> + LIBS="$saved_LIBS"
>> + ])
>> if test $st_cv_mpers = yes; then
>> AC_DEFINE(HAVE_MPERS, [1],
>> [Define to 1 if you have mpers_name mpers support])
> Since st_MPERS code is executed after st_SELINUX, its results can be used
> here. If with_secontexts was set to "no" by st_SELINUX, there is no need
> to bother about selinux runtime here. Also, if st_cv_runtime was set to
> "no" by earlier checks, there is no need to check selinux runtime either.
>
> For example:
>
> AS_IF([test "x$with_secontexts$st_cv_mpers$st_cv_runtime" = xyesyesyes],
> [AC_CACHE_CHECK([whether selinux runtime works with mpers_name personality],
> [st_cv_selinux_runtime],
> [saved_CPPFLAGS="$CPPFLAGS"
> saved_LDFLAGS="$LDFLAGS"
> saved_LIBS="$LIBS"
> CPPFLAGS="$CPPFLAGS libselinux_CPPFLAGS"
> LDFLAGS="$LDFLAGS libselinux_LDFLAGS"
> LIBS="$LIBS libselinux_LIBS"
> AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include <selinux/selinux.h>]],
> [[return 0]])],
> [st_cv_selinux_runtime=yes],
> [st_cv_selinux_runtime=no],
> [st_cv_selinux_runtime=no])
> LIBS="$saved_LIBS"
> LDFLAGS="$saved_LDFLAGS"
> CPPFLAGS="$saved_CPPFLAGS"
> ])
> ],
> [st_cv_selinux_runtime=no])
OK
> diff --git a/src/Makefile.am b/src/Makefile.am
>> index ee9415ea5..e5553918a 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -186,6 +186,7 @@ libstrace_a_SOURCES = \
>> mmap_notify.c \
>> mmap_notify.h \
>> mmsghdr.c \
>> + mntns.c \
>> mount.c \
>> move_mount.c \
>> mpers_type.h \
>> @@ -303,6 +304,7 @@ libstrace_a_SOURCES = \
>> sched_attr.h \
>> scsi.c \
>> seccomp.c \
>> + selinux.h \
>> sendfile.c \
>> sg_io_v3.c \
>> sg_io_v4.c \
>> @@ -406,6 +408,13 @@ strace_LDADD += $(libiberty_LIBS)
>> endif
>> endif
>>
>> +if USE_SELINUX
>> +libstrace_a_SOURCES += selinux.c
>> +strace_CPPFLAGS += $(libselinux_CPPFLAGS)
>> +strace_LDFLAGS += $(libselinux_LDFLAGS)
>> +strace_LDADD += $(libselinux_LIBS)
>> +endif
>> +
>> strace_CPPFLAGS += $(CODE_COVERAGE_CPPFLAGS)
>> strace_CFLAGS += $(CODE_COVERAGE_CFLAGS)
>> strace_LDADD += $(CODE_COVERAGE_LIBS)
> It's an interesting question whether you want to compile mntns.c when
> selinux support is not enabled. Maybe some day in the future mntns.c might
> be used outside USE_SELINUX code, bit currently it looks like we would
> better off without mntns.c when USE_SELINUX is not enabled.
>
> In other words, let's move mntns.c and selinux.h into "if USE_SELINUX"
> case.
OK
>> diff --git a/src/access.c b/src/access.c
>> index 2936242f7..81b78889b 100644
>> --- a/src/access.c
>> +++ b/src/access.c
>> @@ -34,8 +34,10 @@ decode_faccessat(struct tcb *tcp)
>> {
>> /* dirfd */
>> print_dirfd(tcp, tcp->u_arg[0]);
>> +#ifdef USE_SELINUX
>> + tcp->dirfd = (int)(tcp->u_arg[0]);
>> +#endif
>> tprint_arg_next();
>> -
>> decode_access(tcp, 1);
>> }
>> [...]
>> You've added a lot of such tcp->dirfd assignments. Apparently,
>> they are added after every print_dirfd invocation except one (and that one
>> seems to be omitted by mistake). If you want to cache dirfd processed by
>> print_dirfd, it would be much easier if you just moved this assignment
>> inside print_dirfd.
Initially I had this idea as well but then thought it was ugly to have
this state saved in print_dirfd().
Additionally we need to really know when to set the dirfd: there may be
cases where print_dirfd() is called but the next arguments to the
syscall won't use that dirfd as "reference".
Hence it's safer to assign dirfd in the syscall handler itself.
>> diff --git a/src/defs.h b/src/defs.h
>> index 08a293130..673e86539 100644
>> --- a/src/defs.h
>> +++ b/src/defs.h
>> @@ -293,6 +293,17 @@ struct tcb {
>> */
>> unsigned int pid_ns;
>>
>> + /*
>> + * The ID of the MOUNT namespace of this process
>> + * (inode number of /proc/<pid>/ns/mnt)
>> + * (0: not initialized)
>> + */
>> + unsigned int mnt_ns;
>> +
>> +#ifdef USE_SELINUX
>> + int dirfd; /* Use AT_FDCWD for 'not set' */
>> +#endif
>> +
>> struct mmap_cache_t *mmap_cache;
>>
>> /*
> Since mnt_ns shouldn't be used outside USE_SELINUX yet, let's move it
> inside USE_SELINUX.
>
> Also, dirfd seems to be too short, I'd like to rename it to something more
> descriptive, e.g. last_dirfd.
>
OK
>> @@ -1046,8 +1057,13 @@ print_local_array_ex(struct tcb *tcp,
>> extern kernel_ulong_t *
>> fetch_indirect_syscall_args(struct tcb *, kernel_ulong_t addr, unsigned int n_args);
>>
>> +extern unsigned int get_mnt_ns(struct tcb *tcp);
>> +extern unsigned int get_our_mnt_ns(void);
>> +
>> extern void pidns_init(void);
>>
>> +extern const char *pid_to_str(pid_t pid);
>> +
>> /**
>> * Returns the pid of the tracee as present in /proc of the tracer (can be
>> * different from tcp->pid if /proc and the tracer process are in different PID
> You can move these declarations inside USE_SELINUX, but it's not
> necessary.
OK
>> diff --git a/src/mntns.c b/src/mntns.c
>> new file mode 100644
>> index 000000000..ac6cddbf6
>> --- /dev/null
>> +++ b/src/mntns.c
>> @@ -0,0 +1,72 @@
>> +/*
>> + * Copyright (c) 2021 The strace developers.
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>> + */
>> +
>> +#include "defs.h"
>> +
>> +#include <fcntl.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +
>> +#include "largefile_wrappers.h"
>> +#include "xstring.h"
>> +
>> +static unsigned int
>> +get_mnt_ns_of_pid(int pid)
>> +{
>> + unsigned int ns = 0;
>> + char path[PATH_MAX + 1];
>> + xsprintf(path, "/proc/%s/ns/mnt", pid_to_str(pid));
>> +
>> + int fd = open_file(path, O_RDONLY);
>> + if (fd < 0)
>> + return 0;
>> + strace_stat_t st;
>> + if (fstat_fd(fd, &st))
>> + goto out;
>> + ns = st.st_ino;
>> +out:
>> + close(fd);
> Wouldn't it be easier to read if the last 5 lines were written this way:
>
> if (fstat_fd(fd, &st) == 0)
> ns = st.st_ino;
> close(fd);
>
> I'd go even further:
>
> if (fd >= 0) {
> strace_stat_t st;
> if (fstat_fd(fd, &st) == 0)
> ns = st.st_ino;
> close(fd);
> }
OK
>> +
>> + return ns;
>> +}
>> +
>> +/**
>> + * Returns the MOUNT namespace of the tracee
>> + */
>> +unsigned int
>> +get_mnt_ns(struct tcb *tcp)
>> +{
>> + if (tcp->mnt_ns)
>> + return tcp->mnt_ns;
>> +
>> + int proc_pid = 0;
>> + translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
>> +
>> + if (!proc_pid)
>> + return 0;
>> +
>> + tcp->mnt_ns = get_mnt_ns_of_pid(proc_pid);
> Wouldn't it be easier to read if the last 4 lines were written this way:
>
> if (proc_pid)
> tcp->mnt_ns = get_mnt_ns_of_pid(proc_pid);
>
OK
>> +
>> + return tcp->mnt_ns;
>> +}
>> +
>> +/**
>> + * Returns the MOUNT namespace of strace
>> + */
>> +unsigned int
>> +get_our_mnt_ns(void)
>> +{
>> + static unsigned int our_ns = 0;
>> + static bool our_ns_initialised = false;
>> +
>> + if (!our_ns_initialised) {
>> + our_ns = get_mnt_ns_of_pid((int)getpid());
> I don't think this explicit cast is really needed here.
OK
>> + our_ns_initialised = true;
>> + }
>> +
>> + return our_ns;
>> +}
> The rest of this file is OK
>
>> diff --git a/src/pidns.c b/src/pidns.c
>> index dbcf52b91..6221f902b 100644
>> --- a/src/pidns.c
>> +++ b/src/pidns.c
>> @@ -142,7 +142,7 @@ get_cached_proc_pid(unsigned int ns, int ns_pid, enum pid_type type)
>> * Helper function, converts pid to string, or to "self" for pid == 0.
>> * Uses static buffer for operation.
>> */
>> -static const char *
>> +const char *
>> pid_to_str(pid_t pid)
>> {
>> if (!pid)
> OK
>
>> diff --git a/src/selinux.c b/src/selinux.c
>> new file mode 100644
>> index 000000000..0ea9e46ee
>> --- /dev/null
>> +++ b/src/selinux.c
>> @@ -0,0 +1,179 @@
>> +/*
>> + * Copyright (c) 2020-2021 The strace developers.
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>> + */
>> +
>> +#include <fcntl.h>
>> +#include <limits.h>
>> +#include <selinux/selinux.h>
>> +#include <stdlib.h>
>> +
>> +#include "defs.h"
> "defs.h" must be included first.
>
> Also, I prefer to have <stdlib.h> included before <selinux/selinux.h>.
OK
>> +#include "number_set.h"
>> +#include "selinux.h"
>> +#include "xstring.h"
>> +
>> +#ifndef AT_FDCWD
>> +# define AT_FDCWD -100
>> +#endif
>> +
>> +bool selinux_context = false;
>> +bool selinux_context_full = false;
>> +
>> +enum selinux_objtype {
>> + SELINUX_PID,
>> + SELINUX_PATH
>> +};
>> +
>> +typedef struct {
>> + enum selinux_objtype type;
>> + union {
>> + pid_t pid;
>> + char *path;
>> + } obj;
>> +} selinux_obj_t;
>> +
>> +static int getcontext(selinux_obj_t *obj, char **context);
>> +
>> +/*
>> + * Retrieves the SELinux context of the given PID (extracted from the tcb).
>> + * Memory must be freed.
>> + * Returns 0 on success, -1 on failure.
>> + */
>> +int
>> +selinux_getpidcon(struct tcb *tcp, char **context)
>> +{
>> + int proc_pid = 0;
>> + translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
>> + if (!proc_pid)
>> + return -1;
> Let's check selinux_context first and return from this function
> if selinux_context is not enabled.
I don't understand. We need *proc_pid* to get the context, so the
translation must be performed initially.
>> +
>> + selinux_obj_t obj;
>> + obj.type = SELINUX_PID;
>> + obj.obj.pid = (pid_t)proc_pid;
>> +
>> + return getcontext(&obj, context);
>> +}
>> +
>> +/*
>> + * Retrieves the SELinux context of the given path.
>> + * Memory must be freed.
>> + * Returns 0 on success, -1 on failure.
>> + */
>> +int
>> +selinux_getfilecon(struct tcb *tcp, char *path, char **context)
> Let's change "char *path" to "const char *path".
OK
>> +{
>> + /*
>> + * Current limitation: we cannot query the path if we are in different
>> + * mount namespaces.
>> + */
>> + if (get_mnt_ns(tcp) != get_our_mnt_ns())
>> + return -1;
> Let's check selinux_context first and return from this function
> if selinux_context is not enabled.
Here, if we are not in the right context, then getfilecon() will return
false positives, so I think the first thing to check is that we can
indeed retrieve a valid context.
>> +
>> + selinux_obj_t obj;
>> + obj.type = SELINUX_PATH;
>> + obj.obj.path = path;
>> +
>> + if (path[0] == '/')
>> + return getcontext(&obj, context);
>> +
>> + char resolved[PATH_MAX + 1];
>> + char pathtoresolve[2 * PATH_MAX + 2];
>> + int proc_pid = 0;
>> + translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
>> + if (!proc_pid)
>> + return -1;
>> +
>> + /*
>> + * If we have a relative pathname and 'dirfd' == AT_FDCWD, we need to
>> + * prepend by the CWD.
>> + */
>> + if (tcp->dirfd == AT_FDCWD) {
>> + char linkpath[sizeof("/proc/%u/cwd") + sizeof(int)*3];
>> + ssize_t n;
>> +
>> + xsprintf(linkpath, "/proc/%u/cwd", proc_pid);
>> + n = readlink(linkpath, resolved, sizeof(resolved) - 1);
>> + /*
>> + * NB: if buffer is too small, readlink doesn't fail,
>> + * it returns truncated result.
>> + */
>> + if (n == sizeof(resolved) - 1)
>> + return -1;
>> + resolved[n] = '\0';
>> + } else {
>> + if (number_set_array_is_empty(decode_fd_set, 0) ||
> Interesting. This means that --decode-fds should have effect on
> selinux_getfilecon. I wonder why.
From my research, there is no way to get a path from a dirfd, hence we
need implicitly --decode-fds for this to work, since the latter will
have the mapping fd -> pathname for us.
But actually I removed the number_set_array_is_empty() and it just works ...
>> + getfdpath_pid(proc_pid, tcp->dirfd, resolved, sizeof(resolved)) < 0)
>> + return -1;
> getfdpath_pid itself uses translate_pid to translate it's first argument.
>
OK, I'm wrong indeed.
>> + }
>> + xsprintf(pathtoresolve, "%s/%s", resolved, path);
>> + if (realpath(pathtoresolve, resolved) == NULL) {
> Interesting. Why realpath? Would stat_file be enough?
No, because the path to check needs to be absolute, or else cwd of
strace will be used for resolving the context, which is wrong.
>> + /*
>> + * This can happen in multiple cases:
>> + * - there was an error different than ENOENT
>> + * - if path doesn't exist
>> + * - path is below root directory, e.g. root dir and cwd are
>> + * '/home/<user>/chroot' and path is below root directory,
>> + * e.g. '../../bin/ls'
>> + */
>> + return -1;
>> + }
>> + obj.obj.path = resolved;
>> +
>> + return getcontext(&obj, context);
>> +}
>> +
>> +static int
>> +getcontext(selinux_obj_t *obj, char **context)
>> +{
>> + int res;
>> + char *secontext;
>> +
>> + if (!selinux_context)
>> + goto fail;
>> +
>> + switch (obj->type) {
>> + case SELINUX_PID:
>> + res = getpidcon(obj->obj.pid, &secontext);
>> + break;
>> + case SELINUX_PATH:
>> + res = getfilecon(obj->obj.path, &secontext);
>> + break;
>> + default:
>> + /* XXX Assert instead */
>> + goto fail;
> This is the reason why I don't like this enum/case approach:
> it creates unreachable code.
OK changed into if/else
>> + }
>> +
>> + if (res == -1)
>> + goto fail;
>> +
>> + if (selinux_context_full) {
>> + *context = xstrdup(secontext);
>> + } else {
>> + char *saveptr = NULL;
>> + char *secontext_copy = xstrdup(secontext);
>> + const char *token;
>> + int i;
>> +
>> + /*
>> + * We only want to keep the type (3rd field, ':' separator).
>> + */
>> + *context = NULL;
>> + for (token = strtok_r(secontext_copy, ":", &saveptr), i = 0;
>> + token; token = strtok_r(NULL, ":", &saveptr), i++) {
>> + if (i == 2) {
>> + *context = xstrdup(token);
>> + break;
>> + }
>> + }
>> + if (*context == NULL)
>> + *context = xstrdup(secontext);
>> + free(secontext_copy);
>> + }
>> + freecon(secontext);
>> + return 0;
>> +fail:
>> + return -1;
>> +}
> I suggest some rewriting, for example:
>
> #include "defs.h"
>
> #include <fcntl.h>
> #include <limits.h>
> #include <stdlib.h>
> #include <selinux/selinux.h>
>
> #include "number_set.h"
> #include "selinux.h"
> #include "xstring.h"
>
> #ifndef AT_FDCWD
> # define AT_FDCWD -100
> #endif
>
> bool selinux_context = false;
> bool selinux_context_full = false;
>
> static int
> getcontext(int rc, char **secontext, char **result)
> {
> if (rc < 0)
> return rc;
>
> *result = NULL;
> if (!selinux_context_full) {
> char *saveptr = NULL;
> char *secontext_copy = xstrdup(*secontext);
> const char *token;
> unsigned int i;
>
> /*
> * We only want to keep the type (3rd field, ':' separator).
> */
> for (token = strtok_r(secontext_copy, ":", &saveptr), i = 0;
> token; token = strtok_r(NULL, ":", &saveptr), i++) {
> if (i == 2) {
> *result = xstrdup(token);
> break;
> }
> }
> free(secontext_copy);
> }
>
> if (*result == NULL)
> *result = xstrdup(*secontext);
> freecon(*secontext);
> return 0;
> }
>
> /*
> * Retrieves the SELinux context of the given PID (extracted from the tcb).
> * Memory must be freed.
> * Returns 0 on success, -1 on failure.
> */
> int
> selinux_getpidcon(struct tcb *tcp, char **result)
> {
> if (!selinux_context)
> return -1;
>
> int proc_pid = 0;
> translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
> if (!proc_pid)
> return -1;
>
> char *secontext;
> return getcontext(getpidcon(proc_pid, &secontext), &secontext, result);
> }
>
> /*
> * Retrieves the SELinux context of the given path.
> * Memory must be freed.
> * Returns 0 on success, -1 on failure.
> */
> int
> selinux_getfilecon(struct tcb *tcp, const char *path, char **result)
> {
> if (!selinux_context)
> return -1;
>
> /*
> * Current limitation: we cannot query the path if we are in different
> * mount namespaces.
> */
> if (get_mnt_ns(tcp) != get_our_mnt_ns())
> return -1;
>
> char *secontext;
>
> if (path[0] == '/')
> return getcontext(getfilecon(path, &secontext), &secontext, result);
>
> char resolved[PATH_MAX + 1];
>
> /*
> * If we have a relative pathname and 'dirfd' == AT_FDCWD, we need to
> * prepend by the CWD.
> */
> if (tcp->last_dirfd == AT_FDCWD) {
> int proc_pid = 0;
> translate_pid(NULL, tcp->pid, PT_TID, &proc_pid);
> if (!proc_pid)
> return -1;
>
> char linkpath[sizeof("/proc/%u/cwd") + sizeof(int)*3];
> xsprintf(linkpath, "/proc/%u/cwd", proc_pid);
> ssize_t n = readlink(linkpath, resolved, sizeof(resolved) - 1);
> /*
> * NB: if buffer is too small, readlink doesn't fail,
> * it returns truncated result.
> */
> if (n == sizeof(resolved) - 1)
> return -1;
> resolved[n] = '\0';
> } else {
> if (getfdpath_pid(tcp->pid, tcp->last_dirfd,
> resolved, sizeof(resolved)) < 0)
> return -1;
> }
>
> char pathtoresolve[2 * PATH_MAX + 2];
> xsprintf(pathtoresolve, "%s/%s", resolved, path);
> /* XXX Why realpath? Would stat_file be enough? */
> if (realpath(pathtoresolve, resolved) == NULL) {
> /*
> * This can happen in multiple cases:
> * - there was an error different than ENOENT
> * - if path doesn't exist
> * - path is below root directory, e.g. root dir and cwd are
> * '/home/<user>/chroot' and path is below root directory,
> * e.g. '../../bin/ls'
> */
> return -1;
> }
>
> return getcontext(getfilecon(resolved, &secontext), &secontext, result);
> }
OK, smarter code indeed.
> @@ -896,6 +922,13 @@ alloctcb(int pid)
>> tcp->pid = pid;
>> #if SUPPORTED_PERSONALITIES > 1
>> tcp->currpers = current_personality;
>> +#endif
>> +#ifdef USE_SELINUX
>> +#ifndef AT_FDCWD
>> +# define AT_FDCWD>------100
>> +#endif
> What is this? :)
Well, that was defined multiple times in the pre 5.11 sources. I removed
this right now.
>
>> @@ -3220,6 +3275,9 @@ next_event(void)
>> if (!tcp)
>> goto next_event_wait_next;
>> }
>> +#ifdef USE_SELINUX
>> + tcp->dirfd = AT_FDCWD;
>> +#endif
>>
>> if (cflag) {
>> tcp->stime.tv_sec = ru.ru_stime.tv_sec;
> I'm not sure this is the most optimal place to reset tcp->last_dirfd.
> For example, if you want to assign it on entering syscall and use it on
> exiting (e.g. in case of getdents below), this is not the right place
> to reset it.
Actually this has to be done in trace_syscall() upon entering the syscall.
>> diff --git a/src/util.c b/src/util.c
>> index 9cb555ecb..509b12f7f 100644
>> --- a/src/util.c
>> +++ b/src/util.c
>> @@ -26,6 +26,7 @@
>> #include "largefile_wrappers.h"
>> #include "number_set.h"
>> #include "print_utils.h"
>> +#include "selinux.h"
>> #include "static_assert.h"
>> #include "string_to_uint.h"
>> #include "xlat.h"
>> @@ -664,6 +665,13 @@ printfd_pid(struct tcb *tcp, pid_t pid, int fd)
>>
>> printed:
>> tprints(">");
>> +#ifdef USE_SELINUX
>> + char *context;
>> + if (!selinux_getfilecon(tcp, path, &context)) {
>> + tprintf(" [%s]", context);
>> + free(context);
>> + }
>> +#endif
>> } else {
>> tprintf("%d", fd);
>> }
>> @@ -959,6 +967,14 @@ printpathn(struct tcb *const tcp, const kernel_ulong_t addr, unsigned int n)
>> else {
>> path[n++] = !nul_seen;
>> print_quoted_cstring(path, n);
>> +
>> +#ifdef USE_SELINUX
>> + char *context;
>> + if (nul_seen && !selinux_getfilecon(tcp, path, &context)) {
>> + tprintf(" [%s]", context);
>> + free(context);
>> + }
>> +#endif
>> }
>>
>> return nul_seen;
> This is OK, but there are few places in the code where printpathn is used
> in different contexts where this wouldn't be appropriate:
> - all readdir/getdents contexts, printpathn is used there relative to the
> corresponding directory, a solution is to assign tcp->last_dirfd inside
> SYS_FUNC(readdir) and xgetdents;
> - memfd_create and sock_ioctl use printpathn to print something that's not
> a filesystem object at all, probably they should use printstrn instead.
I missed some cases indeed, I was checking print_dirfd() callers only.
I however didn't replace printpathn() for memfd_create and sock_ioctl by
printstrn() because it should be another commit I guess.
>
>> diff --git a/strace.spec.in b/strace.spec.in
>> index c8f281f18..b3a2895d4 100644
>> --- a/strace.spec.in
>> +++ b/strace.spec.in
>> @@ -29,11 +29,13 @@ BuildRequires: pkgconfig(bluez)
>> # Install binutils-devel to enable symbol demangling.
>> %if 0%{?fedora} >= 20 || 0%{?centos} >= 6 || 0%{?rhel} >= 6
>> %define buildrequires_stacktrace BuildRequires: elfutils-devel binutils-devel
>> +%define buildrequires_selinux BuildRequires: libselinux-devel
>> %endif
>> %if 0%{?suse_version} >= 1100
>> %define buildrequires_stacktrace BuildRequires: libdw-devel binutils-devel
>> %endif
>> %{?buildrequires_stacktrace}
>> +%{?buildrequires_selinux}
>>
>> # OBS compatibility
>> %{?!buildroot:BuildRoot: %_tmppath/buildroot-%name-%version-%release}
> Maybe opensuse also provides libselinux-devel that could be used here.
Indeed there is. Actually I tested on RHEL and Fedora only.
>
> I haven't reviewed the tests yet, just tried to build and run them.
> I needed to apply the following change to make them pass:
>
> diff --git a/tests/gen_tests.in b/tests/gen_tests.in
> index 3f41959a9..c674c9891 100644
> --- a/tests/gen_tests.in
> +++ b/tests/gen_tests.in
> @@ -10,8 +10,8 @@ _newselect-P -e trace=_newselect -P /dev/full 9>>/dev/full
> accept -a22
> accept4 -a37
> access -a30 --trace-path=access_sample
> -access--secontext --secontext --trace-path=access_sample -e trace=access
> -access--secontext_full --secontext=full --trace-path=access_sample -e trace=access
> +access--secontext -a30 --secontext --trace-path=access_sample -e trace=access
> +access--secontext_full -a30 --secontext=full --trace-path=access_sample -e trace=access
OK, I wasn't sure about the need to "-aXX" option, so removed all this
because I didn't see any difference on my systems.
> Also please note that you can use asprintf and functions from xmalloc.h
> in tests, not need to use complicated malloc's followed by asserts.
OK
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ldv-s-comments-integration.patch
Type: text/x-patch
Size: 25860 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20210315/d2818b2c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20210315/d2818b2c/attachment-0001.bin>
More information about the Strace-devel
mailing list