Proposing SELinux support in strace
Dmitry V. Levin
ldv at altlinux.org
Sat Nov 21 01:30:40 UTC 2020
On Wed, Nov 18, 2020 at 11:44:40AM +0100, Renaud Métrich wrote:
> The code is now complete,
Thanks. As I wouldn't like to move the whole review process to
a proprietary platform, let's pretend you posted the patch here.
> Makefile.am | 8 +++
> configure.ac | 2 +
> defs.h | 3 ++
> m4/st_selinux.m4 | 68 ++++++++++++++++++++++++
> selinux.c | 86 +++++++++++++++++++++++++++++++
> selinux.h | 24 +++++++++
> strace.c | 52 ++++++++++++++++++-
> strace.spec.in | 2 +
> tests/Makefile.am | 2 +
> tests/strace--secontext.test | 46 +++++++++++++++++
> tests/strace--secontext_full.test | 47 +++++++++++++++++
> tests/strace-V.test | 4 +-
> util.c | 21 ++++++++
> 13 files changed, 363 insertions(+), 2 deletions(-)
> create mode 100644 m4/st_selinux.m4
> create mode 100644 selinux.c
> create mode 100644 selinux.h
> create mode 100755 tests/strace--secontext.test
> create mode 100755 tests/strace--secontext_full.test
>
> diff --git a/Makefile.am b/Makefile.am
> index 723460f2e..b28d501f1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -380,6 +380,7 @@ libstrace_a_SOURCES = \
> xmalloc.c \
> xmalloc.h \
> xstring.h \
> + selinux.h \
Please keep this list sorted.
> $(TYPES_HEADER_FILES) \
> $(strace_SOURCES_check) \
> # end of libstrace_a_SOURCES
> @@ -408,6 +409,13 @@ strace_LDADD += $(libiberty_LIBS)
> endif
> endif
>
> +if USE_SELINUX
> +libstrace_a_SOURCES += selinux.c
> +strace_CPPFLAGS += $(libselinux_CPPFLAGS)
> +strace_LDFLAGS += $(libselinux_CPPFLAGS)
Did you mean libselinux_LDFLAGS?
> +strace_LDADD += $(libselinux_LIBS)
> +endif
> +
> @CODE_COVERAGE_RULES@
> CODE_COVERAGE_BRANCH_COVERAGE = 1
> CODE_COVERAGE_GENHTML_OPTIONS = $(CODE_COVERAGE_GENHTML_OPTIONS_DEFAULT) \
> diff --git a/configure.ac b/configure.ac
> index 4e7bc2a89..d0d3a36be 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -764,6 +764,8 @@ AC_CHECK_TOOL([READELF], [readelf])
>
> st_STACKTRACE
>
> +st_SELINUX
> +
> if test "$arch" = mips && test "$no_create" != yes; then
> mkdir -p linux/mips
> if $srcdir/linux/mips/genstub.sh \
> diff --git a/defs.h b/defs.h
> index 7151e90cb..8356ec378 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -489,6 +489,7 @@ extern cflag_t cflag;
> extern bool Tflag;
> extern int Tflag_scale;
> extern int Tflag_width;
> +extern bool selinux_context;
> extern bool iflag;
> extern bool count_wallclock;
> extern unsigned int pidns_translation;
> @@ -522,6 +523,8 @@ extern unsigned os_release;
> # define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
>
> extern int read_int_from_file(const char *, int *);
> +extern int selinux_getpidcon(pid_t pid, char **context);
> +extern int selinux_getfilecon(const char *path, char **context);
>
> extern void set_sortby(const char *);
> extern int set_overhead(const char *);
Given that you introduce a separate header for this,
I don't think you need to touch defs.h at all.
> diff --git a/m4/st_selinux.m4 b/m4/st_selinux.m4
> new file mode 100644
> index 000000000..309b4a096
> --- /dev/null
> +++ b/m4/st_selinux.m4
> @@ -0,0 +1,68 @@
> +#!/usr/bin/m4
> +#
> +# Copyright (c) 2020 The strace developers.
> +# All rights reserved.
> +#
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +
> +AC_DEFUN([st_SELINUX], [dnl
> +
> +AC_ARG_WITH([libselinux],
> + [AS_HELP_STRING([--with-libselinux],
> + [use libselinux to collect security contexts])],
> + [case "${withval}" in
> + yes|no|check) ;;
> + *) with_libselinux=yes
> + libselinux_CPPFLAGS="-I${withval}/include"
> + libselinux_LDFLAGS="-L${withval}/lib" ;;
> + esac],
> + [with_libselinux=check]
> +)
> +
> +libselinux_CPPFLAGS=
> +libselinux_LDFLAGS=
> +libselinux_LIBS=
The initialization of libselinux_* after their first potential use
looks really odd.
> +use_libselinux=no
> +
> +AS_IF([test "x$with_libselinux" != xno],
> + [saved_CPPFLAGS="$CPPFLAGS"
> + CPPFLAGS="$CPPFLAGS $libselinux_CPPFLAGS"
> + found_selinux_h=no
> + AC_CHECK_HEADERS([selinux.h selinux/selinux.h],
Do you really want to support selinux.h header? If yes, than you cannot
use selinux.h as the name for the strace header file, and you have to
check which one of these two headers exist. If unsure, just stick with
selinux/selinux.h.
> + [found_selinux_h=yes])
> + CPPFLAGS="$saved_CPPFLAGS"
> + AS_IF([test "x$found_selinux_h" = xyes],
> + [saved_LDFLAGS="$LDFLAGS"
> + LDFLAGS="$LDFLAGS $libselinux_LDFLAGS"
> + AC_CHECK_LIB([selinux],[getpidcon],
> + [libselinux_LIBS="-lselinux"
> + use_libselinux=yes
> + ],
> + [if test "x$with_libselinux" != xcheck; then
> + AC_MSG_FAILURE([failed to find getpidcon in libselinux])
> + fi
> + ]
> + )
> + LDFLAGS="$saved_LDFLAGS"
> + ],
> + [if test "x$with_libselinux" != xcheck; then
> + AC_MSG_FAILURE([failed to find selinux.h])
> + fi
> + ]
> + )
> + ]
> +)
> +
> +AC_MSG_CHECKING([whether to enable security contexts support])
> +AS_IF([test "x$use_libselinux" = xyes],
> + [AC_DEFINE([USE_SELINUX], [1],
> + [Define to enable SELinux security contexts support])
> + AC_SUBST(libselinux_LIBS)
> + AC_SUBST(libselinux_LDFLAGS)
> + AC_SUBST(libselinux_CPPFLAGS)
> + AC_MSG_RESULT([yes])],
> + [AC_MSG_RESULT([no])])
> +
> +AM_CONDITIONAL([USE_SELINUX], [test "x$use_libselinux" = xyes])
> +
> +])
> diff --git a/selinux.c b/selinux.c
> new file mode 100644
> index 000000000..e83594d2d
> --- /dev/null
> +++ b/selinux.c
> @@ -0,0 +1,86 @@
> +/*
> + * Copyright (c) 2020 The strace developers.
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#include <selinux/selinux.h>
> +
> +#include "defs.h"
> +#include "selinux.h"
"defs.h" should be included first because the first thing it includes
is "config.h" which in turn must be the first included header.
> +
> +bool selinux_context = false;
> +bool selinux_context_typeonly = true;
I suggest changing selinux_context_typeonly to selinux_context_full
and assigning it to false.
> +
> +/*
> + * Retrieves the SELinux context of the given PID.
> + * Memory must be freed.
> + * Returns 0 on success, -1 on failure.
> + */
> +int
> +selinux_getpidcon(pid_t pid, char **context)
> +{
> + char *secontext;
> + if (selinux_context && !getpidcon(pid, &secontext)) {
> + if (selinux_context_typeonly) {
> + char *saveptr = NULL;
> + const char *token;
> + int i;
> + const char *ret = "???";
> +
> + /*
> + * We only want to keep the type (3rd field, ':' separator).
> + */
> + for (token = strtok_r(secontext, ":", &saveptr), i = 0;
> + token; token = strtok_r(NULL, ":", &saveptr), i++) {
> + if (i == 2) {
> + ret = token;
> + break;
> + }
> + }
> + *context = xstrdup(ret);
> + } else {
> + *context = xstrdup(secontext);
> + }
> + freecon(secontext);
> + return 0;
> + }
> + return -1;
> +}
> +
> +/*
> + * Retrieves the SELinux context of the given PATH.
> + * Memory must be freed.
> + * Returns 0 on success, -1 on failure.
> + */
> +int
> +selinux_getfilecon(const char *path, char **context)
> +{
> + char *secontext;
> + if (getfilecon(path, &secontext) != -1) {
> + if (selinux_context_typeonly) {
> + char *saveptr = NULL;
> + const char *token;
> + int i;
> + const char *ret = "???";
> +
> + /*
> + * We only want to keep the type (3rd field, ':' separator).
> + */
> + for (token = strtok_r(secontext, ":", &saveptr), i = 0;
> + token; token = strtok_r(NULL, ":", &saveptr), i++) {
> + if (i == 2) {
> + ret = token;
> + break;
> + }
> + }
> + *context = xstrdup(ret);
> + } else {
> + *context = xstrdup(secontext);
> + }
> + freecon(secontext);
> + return 0;
> + }
> + return -1;
> +}
The only difference between selinux_getpidcon and selinux_getfilecon
implementations is just one line of code:
- if (selinux_context && !getpidcon(pid, &secontext)) {
+ if (getfilecon(path, &secontext) != -1) {
I urge you to create a helper function and move everything except
the getpidcon/getfilecon invocation there.
I suggest to check selinux_context both in selinux_getpidcon and
selinux_getfilecon, this way you won't have to check selinux_context
in every place where selinux_getpidcon or selinux_getfilecon is called.
> diff --git a/selinux.h b/selinux.h
> new file mode 100644
> index 000000000..140d98ef6
> --- /dev/null
> +++ b/selinux.h
> @@ -0,0 +1,24 @@
> +/*
> + * SELinux interface.
> + *
> + * Copyright (c) 2020 The strace developers.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#ifndef STRACE_SELINUX_H
> +# define STRACE_SELINUX_H
> +
> +#ifdef USE_SELINUX
> +
> +#include "defs.h"
Since USE_SELINUX is a macro defined in config.h, you have two variants:
- assume defs.h is already included;
- include defs.h first, then check for USE_SELINUX.
> +
> +extern bool selinux_context;
> +extern bool selinux_context_typeonly;
> +
> +int selinux_getpidcon(pid_t pid, char **context);
> +int selinux_getfilecon(const char *path, char **context);
> +
> +#endif
> +
> +#endif /* !STRACE_SELINUX_H */
> diff --git a/strace.c b/strace.c
> index 2897195c9..d83ae4cf0 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -40,6 +40,7 @@
> #include "xstring.h"
> #include "delay.h"
> #include "wait.h"
> +#include "selinux.h"
>
> /* In some libc, these aren't declared. Do it ourself: */
> extern char **environ;
> @@ -240,6 +241,9 @@ print_version(void)
> " no-mx32-mpers"
> # endif
> #endif /* SUPPORTED_PERSONALITIES > 2 */
> +#ifdef USE_SELINUX
> + " secontext"
> +#endif
> "";
>
> printf("%s -- version %s\n"
> @@ -258,12 +262,18 @@ usage(void)
> # define K_OPT "k"
> #else
> # define K_OPT ""
> +#endif
> +#ifdef USE_SELINUX
> +# define SELINUX_OPT "[--secontext[=full]]\n"
> +#else
> +# define SELINUX_OPT ""
> #endif
>
> printf("\
> Usage: strace [-ACdffhi" K_OPT "qqrtttTvVwxxyyzZ] [-I N] [-b execve] [-e EXPR]...\n\
> [-a COLUMN] [-o FILE] [-s STRSIZE] [-X FORMAT] [-O OVERHEAD]\n\
> - [-S SORTBY] [-P PATH]... [-p PID]... [-U COLUMNS] [--seccomp-bpf]\n\
> + [-S SORTBY] [-P PATH]... [-p PID]... [-U COLUMNS] [--seccomp-bpf]\n"\
> + SELINUX_OPT "\
> { -p PID | [-DDD] [-E VAR=VAL]... [-u USERNAME] PROG [ARGS] }\n\
> or: strace -c[dfwzZ] [-I N] [-b execve] [-e EXPR]... [-O OVERHEAD]\n\
> [-S SORTBY] [-P PATH]... [-p PID]... [-U COLUMNS] [--seccomp-bpf]\n\
> @@ -439,6 +449,14 @@ Miscellaneous:\n\
> -d, --debug enable debug output to stderr\n\
> -h, --help print help message\n\
> --seccomp-bpf enable seccomp-bpf filtering\n\
> +"
> +#ifdef USE_SELINUX
> +"\
> + --secontext[=full]\n\
> + print SELinux contexts (type only unless 'full' is specified)\n\
> +"
> +#endif
> +"\
> -V, --version print version\n\
> "
> /* ancient, no one should use it
> @@ -747,6 +765,10 @@ set_current_tcp(const struct tcb *tcp)
> void
> printleader(struct tcb *tcp)
> {
> +#ifdef USE_SELINUX
> + char *context;
> +#endif
> +
Feel free to define the local variable right before the first use.
> /* If -ff, "previous tcb we printed" is always the same as current,
> * because we have per-tcb output files.
> */
> @@ -777,6 +799,13 @@ printleader(struct tcb *tcp)
> else if (nprocs > 1 && !outfname)
> tprintf("[pid %5u] ", tcp->pid);
>
> +#ifdef USE_SELINUX
> + if (selinux_context && !selinux_getpidcon(tcp->pid, &context)) {
selinux_getpidcon also checks selinux_context, no need to do it twice.
> + tprintf("[%s] ", context);
> + free(context);
> + }
> +#endif
> +
> if (tflag_format) {
> struct timespec ts;
> clock_gettime(CLOCK_REALTIME, &ts);
> @@ -2033,6 +2062,9 @@ init(int argc, char *argv[])
> GETOPT_OUTPUT_SEPARATELY,
> GETOPT_TS,
> GETOPT_PIDNS_TRANSLATION,
> +#ifdef USE_SELINUX
> + GETOPT_SELINUX_CONTEXT,
> +#endif
>
> GETOPT_QUAL_TRACE,
> GETOPT_QUAL_ABBREV,
> @@ -2089,6 +2121,9 @@ init(int argc, char *argv[])
> { "failed-only", no_argument, 0, 'Z' },
> { "failing-only", no_argument, 0, 'Z' },
> { "seccomp-bpf", no_argument, 0, GETOPT_SECCOMP },
> +#ifdef USE_SELINUX
> + { "secontext", optional_argument, 0, GETOPT_SELINUX_CONTEXT },
> +#endif
>
> { "trace", required_argument, 0, GETOPT_QUAL_TRACE },
> { "abbrev", required_argument, 0, GETOPT_QUAL_ABBREV },
> @@ -2317,6 +2352,17 @@ init(int argc, char *argv[])
> case GETOPT_SECCOMP:
> seccomp_filtering = true;
> break;
> +#ifdef USE_SELINUX
> + case GETOPT_SELINUX_CONTEXT:
> + selinux_context = true;
> + if (optarg) {
> + if (!strcmp(optarg, "full"))
> + selinux_context_typeonly = false;
If you rename selinux_context_typeonly to selinux_context_full,
this would check to selinux_context_full = true
which makes thes code more readable.
> + else
> + error_opt_arg(c, lopt, optarg);
> + }
> + break;
> +#endif
> case GETOPT_QUAL_TRACE:
> qualify_trace(optarg);
> break;
> @@ -2499,6 +2545,10 @@ init(int argc, char *argv[])
> if (!number_set_array_is_empty(decode_fd_set, 0))
> error_msg("-y/--decode-fds has no effect "
> "with -c/--summary-only");
> +#ifdef USE_SELINUX
> + if (selinux_context)
> + error_msg("--secontext has no effect with -c");
Please make the diagnostics consistent (use -c/--summary-only instead of
plain -c).
> +#endif
> }
>
> if (!outfname) {
> diff --git a/strace.spec.in b/strace.spec.in
> index 27cdc1b89..8b38cf7a9 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}
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 5ea46a7d1..4fd8fff35 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -485,6 +485,8 @@ MISC_TESTS = \
> strace-t.test \
> strace-tt.test \
> strace-ttt.test \
> + strace--secontext_full.test \
> + strace--secontext.test \
> termsig.test \
> threads-execve.test \
> umovestr_cached.test \
> diff --git a/tests/strace--secontext.test b/tests/strace--secontext.test
> new file mode 100755
> index 000000000..aba8c553b
> --- /dev/null
> +++ b/tests/strace--secontext.test
> @@ -0,0 +1,46 @@
> +#!/bin/bash
Why bash? Other tests use /bin/sh.
> +#
> +# Check --secontext option.
> +#
> +# Copyright (c) 2020 The strace developers.
> +# All rights reserved.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +. "${srcdir=.}/init.sh"
> +
> +run_prog_skip_if_failed sh -c "$STRACE -V | grep -qw selinux" > /dev/null 2>&1
Shouldn't it check for "secontext" instead?
Anyway, this is fragile, please use a check similar to the one
tests/PTRACE_SEIZE.sh does, e.g.
$STRACE -V > "$OUT"
grep -Fwq secontext < "$OUT" ||
skip_ "secontext is not enabled"
> +check_prog sed
> +
> +# We execute "sleep" under strace and expect the following output:
> +# - execve(sleep) executes in the context of the shell
> +# - only the context type is printed
> +
> +run_prog sleep 0
We have a local sleep executable, please use
run_prog ../sleep 0
instead.
> +run_strace --secontext -eopenat,execve $args
> +
> +filecontext() {
> + file="$1"
> + ls -Z $file | awk '{ print $1 }'
Please check that ls supports -Z option.
> +}
> +
> +typeonly() {
> + sed 's/.*:\([a-z_]*_t\):.*/\1/'
> +}
> +
> +processctx=$(id -Z | typeonly)
Please check that id -Z works.
> +
> +cat >> "$EXP" << __EOF__
> +\\[$processctx\\] execve\\("$(which sleep)" \\[$(filecontext $(which sleep) | typeonly)\\], .*
> +__EOF__
> +
> +
> +for lib in "/etc/ld.so.cache" "/lib64/libc.so.6"; do
This is fragile, and /lib64/libc.so.6 is very non-portable.
Do you really need to use them?
> + if [ -e "$lib" ]; then
> + cat >> "$EXP" << __EOF__
> +\\[$processctx\\] openat\(AT_FDCWD, "$(echo "$lib" | sed 's/\./\\./g')" \\[$(filecontext "$lib" | typeonly)\\], .*
> +__EOF__
This is very non-portable, you cannot assume that libc will use openat
system call.
> + fi
> +done
You can choose a more easily traceable syscall and use it instead.
strace tests already have a lot of sample executables that could be used
for these purposes, e.g. fchmod and fchmodat. You can even re-use their
expected output if you like.
> +
> +match_grep "$LOG" "$EXP"
match_grep based tests are usually more fragile than match_diff based,
please try to design a test that uses match_diff.
> diff --git a/tests/strace--secontext_full.test b/tests/strace--secontext_full.test
> new file mode 100755
> index 000000000..129103d40
> --- /dev/null
> +++ b/tests/strace--secontext_full.test
> @@ -0,0 +1,47 @@
> +#!/bin/bash
> +#
> +# Check --secontext=full option.
> +#
> +# Copyright (c) 2020 The strace developers.
> +# All rights reserved.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +. "${srcdir=.}/init.sh"
> +
> +run_prog_skip_if_failed sh -c "$STRACE -V | grep -qw selinux" > /dev/null 2>&1
> +check_prog sed
> +check_prog runcon
> +
> +# We execute "runcon -t initrc_t ... sleep" under strace and expect the
> +# following output:
> +# - runcon executes in the context of the shell (e.g. 'unconfined_t')
> +# - execve(sleep) executes in the context of the shell
> +# - sleep switches to 'initrc_t' after execve
> +
> +run_prog runcon -u system_u -r system_r -t initrc_t sleep 0
> +run_strace --secontext=full -eopenat,execve $args
> +
> +filecontext() {
> + file="$1"
> + ls -Z $file | awk '{ print $1 }'
> +}
> +
> +runconctx=$(id -Z)
> +sleepctx=$(id -Z | sed 's/.*:[a-z]*_t:\(.*\)/system_u:system_r:initrc_t:\1/')
> +
> +cat >> "$EXP" << __EOF__
> +\\[$runconctx\\] execve\\("$(which runcon)" \\[$(filecontext $(which runcon))\\], .*
> +\\[$runconctx\\] execve\\("$(which sleep)" \\[$(filecontext $(which sleep))\\], .*
> +__EOF__
> +
> +for lib in "/etc/ld.so.cache" "/lib64/libc.so.6"; do
> + if [ -e "$lib" ]; then
> + cat >> "$EXP" << __EOF__
> +\\[$runconctx\\] openat\\(AT_FDCWD, "$(echo "$lib" | sed 's/\./\\./g')" \\[$(filecontext "$lib")\\], .*
> +\\[$sleepctx\\] openat\\(AT_FDCWD, "$(echo "$lib" | sed 's/\./\\./g')" \\[$(filecontext "$lib")\\], .*
> +__EOF__
> + fi
> +done
> +
> +match_grep "$LOG" "$EXP"
I haven't looked into this, but most of comments about the other test
should apply here as well.
> diff --git a/tests/strace-V.test b/tests/strace-V.test
> index 30f10808c..b5c620e9a 100755
> --- a/tests/strace-V.test
> +++ b/tests/strace-V.test
> @@ -33,7 +33,9 @@ aarch64|powerpc64|s390x|sparc64|tile|x32)
> ;;
> esac
>
> -features="${option_unwind}${option_demangle}${option_m32}${option_mx32}"
> +option_secontext=$(get_config_option USE_SELINUX " secontext")
> +
> +features="${option_unwind}${option_demangle}${option_m32}${option_mx32}${option_secontext}"
> [ -n "$features" ] || features=" (none)"
>
> cat > "$EXP" << __EOF__
> diff --git a/util.c b/util.c
> index 481144bf0..559c481c9 100644
> --- a/util.c
> +++ b/util.c
> @@ -22,6 +22,7 @@
> # include <sys/xattr.h>
> #endif
> #include <sys/uio.h>
> +#include <dlfcn.h>
This looks like a leftover from an old revision,
I don't think you need it any longer.
> #include "largefile_wrappers.h"
> #include "number_set.h"
> @@ -30,6 +31,7 @@
> #include "string_to_uint.h"
> #include "xlat.h"
> #include "xstring.h"
> +#include "selinux.h"
>
> const struct xlat_data *
> find_xlat_val_ex(const struct xlat_data *items, const char *s, size_t num_items,
> @@ -632,6 +634,9 @@ printpidfd(pid_t pid_of_fd, int fd, const char *path)
> void
> printfd_pid(struct tcb *tcp, pid_t pid, int fd)
> {
> +#ifdef USE_SELINUX
> + char *context;
> +#endif
Feel free to define the local variable right before the first use.
> char path[PATH_MAX + 1];
> if (pid > 0 && !number_set_array_is_empty(decode_fd_set, 0)
> && getfdpath_pid(pid, fd, path, sizeof(path)) >= 0) {
> @@ -650,6 +655,12 @@ printfd_pid(struct tcb *tcp, pid_t pid, int fd)
>
> printed:
> tprints(">");
> +#ifdef USE_SELINUX
> + if (selinux_context && !selinux_getfilecon(path, &context)) {
Assuming that selinux_getfilecon also checks selinux_context,
no need to do it twice.
> + tprintf(" [%s]", context);
> + free(context);
> + }
> +#endif
> } else {
> tprintf("%d", fd);
> }
> @@ -927,6 +938,9 @@ int
> printpathn(struct tcb *const tcp, const kernel_ulong_t addr, unsigned int n)
> {
> char path[PATH_MAX];
> +#ifdef USE_SELINUX
> + char *context;
> +#endif
Feel free to define the local variable right before the first use.
> int nul_seen;
>
> if (!addr) {
> @@ -945,6 +959,13 @@ 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
> + if (selinux_context && !selinux_getfilecon(path, &context)) {
Assuming that selinux_getfilecon also checks selinux_context,
no need to do it twice.
> + tprintf(" [%s]", context);
> + free(context);
> + }
> +#endif
> }
>
> return nul_seen;
By the way, is it correct to hook selinux_getfilecon into printpathn?
Also, do you want to display secontext associated with file descriptors?
--
ldv
More information about the Strace-devel
mailing list