diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a1ca43ae4..95f8000bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,7 @@ jobs: automake \ autoconf \ libc6-dev \ + libcap2-bin \ gdb \ docbook \ docbook-xsl \ diff --git a/CODSPEED-CHANGELOG.md b/CODSPEED-CHANGELOG.md index 229119c0e..7e074a9b1 100644 --- a/CODSPEED-CHANGELOG.md +++ b/CODSPEED-CHANGELOG.md @@ -95,6 +95,23 @@ cfni=??? ← Leaving inlined function cfn=printf ``` +### Run executables with file capabilities: `--allow-file-caps` + +**Feature**: Added `--allow-file-caps=patt1,patt2,...` to run executables that carry file capabilities, which Valgrind otherwise refuses. + +**Motivation**: Valgrind loads the client executable itself, so the kernel never grants it its file capabilities, and Valgrind refuses to run such executables (`Permission denied`, exit code 126) rather than run them without the capabilities they expect. Some executables carry capabilities that only part of their code needs, and run fine without them. + +**How it works**: +- An executable whose name matches one of the patterns is run although it carries file capabilities. It runs without them. +- Patterns use the same syntax as `--trace-children-skip`: comma-separated, with `?` and `*` wildcards. +- The option applies to the client executable and to traced children. +- Setuid and setgid executables are still refused. + +**Usage**: +```bash +valgrind --tool=callgrind --trace-children=yes --allow-file-caps=/usr/local/bin/my-tool ./your_program +``` + ### Callgrind: Object-Level Function Skipping **Feature**: Added `--obj-skip=` command-line option to exclude entire objects (shared libraries or executables) from profiling. diff --git a/callgrind/tests/Makefile.am b/callgrind/tests/Makefile.am index a7b65c7ad..48d1bcf59 100644 --- a/callgrind/tests/Makefile.am +++ b/callgrind/tests/Makefile.am @@ -4,7 +4,7 @@ include $(top_srcdir)/Makefile.tool-tests.am SUBDIRS = . DIST_SUBDIRS = . -dist_noinst_SCRIPTS = filter_stderr filter_inline +dist_noinst_SCRIPTS = filter_stderr filter_inline make_capped_true EXTRA_DIST = \ ann1.post.exp ann1.stderr.exp ann1.vgtest \ @@ -13,6 +13,10 @@ EXTRA_DIST = \ register_desc.vgtest register_desc.stderr.exp register_desc.post.exp \ register_desc_threads.vgtest register_desc_threads.stderr.exp \ register_desc_threads.post.exp \ + allow_file_caps.vgtest allow_file_caps.stderr.exp \ + allow_file_caps.stdout.exp \ + allow_file_caps_refused.vgtest allow_file_caps_refused.stderr.exp \ + allow_file_caps_refused.stdout.exp \ find_debuginfo.vgtest find_debuginfo.stderr.exp find_debuginfo.post.exp \ runtime_obj_skip_py.vgtest runtime_obj_skip_py.stderr.exp runtime_obj_skip_py.post.exp \ runtime_obj_skip_py.py runtime_obj_skip_py_shim.c \ diff --git a/callgrind/tests/allow_file_caps.stderr.exp b/callgrind/tests/allow_file_caps.stderr.exp new file mode 100644 index 000000000..e69de29bb diff --git a/callgrind/tests/allow_file_caps.stdout.exp b/callgrind/tests/allow_file_caps.stdout.exp new file mode 100644 index 000000000..9200c2a7b --- /dev/null +++ b/callgrind/tests/allow_file_caps.stdout.exp @@ -0,0 +1 @@ +exit=0 diff --git a/callgrind/tests/allow_file_caps.vgtest b/callgrind/tests/allow_file_caps.vgtest new file mode 100644 index 000000000..f254f0782 --- /dev/null +++ b/callgrind/tests/allow_file_caps.vgtest @@ -0,0 +1,5 @@ +prereq: ./make_capped_true +prog-asis: /bin/sh +args: -c './capped_true; echo exit=$?' +vgopts: -q --trace-children=yes --allow-file-caps=*/capped_true --callgrind-out-file=callgrind.out.allow_file_caps.%p +cleanup: rm -f capped_true callgrind.out.allow_file_caps.* diff --git a/callgrind/tests/allow_file_caps_refused.stderr.exp b/callgrind/tests/allow_file_caps_refused.stderr.exp new file mode 100644 index 000000000..850f6a8ae --- /dev/null +++ b/callgrind/tests/allow_file_caps_refused.stderr.exp @@ -0,0 +1,4 @@ + +Warning: Can't execute setuid/setgid/setcap executable: ./capped_true +Possible workaround: remove --trace-children=yes, if in effect + diff --git a/callgrind/tests/allow_file_caps_refused.stdout.exp b/callgrind/tests/allow_file_caps_refused.stdout.exp new file mode 100644 index 000000000..c35f59a8f --- /dev/null +++ b/callgrind/tests/allow_file_caps_refused.stdout.exp @@ -0,0 +1 @@ +exit=126 diff --git a/callgrind/tests/allow_file_caps_refused.vgtest b/callgrind/tests/allow_file_caps_refused.vgtest new file mode 100644 index 000000000..7195f37d3 --- /dev/null +++ b/callgrind/tests/allow_file_caps_refused.vgtest @@ -0,0 +1,5 @@ +prereq: ./make_capped_true +prog-asis: /bin/sh +args: -c './capped_true 2>/dev/null; echo exit=$?' +vgopts: -q --trace-children=yes --allow-file-caps=*/other --callgrind-out-file=callgrind.out.allow_file_caps_refused.%p +cleanup: rm -f capped_true callgrind.out.allow_file_caps_refused.* diff --git a/callgrind/tests/make_capped_true b/callgrind/tests/make_capped_true new file mode 100755 index 000000000..b5491dbd9 --- /dev/null +++ b/callgrind/tests/make_capped_true @@ -0,0 +1,14 @@ +#! /bin/sh + +# Creates capped_true, a copy of tests/true carrying a file capability. +# Without root or passwordless sudo, an unprivileged user namespace can +# set it on a file we own. Exits 1, so the test is skipped, when none of +# them can. + +dir=`dirname $0` +cp "$dir/../../tests/true" capped_true || exit 1 + +setcap cap_net_raw+ep capped_true 2>/dev/null || + sudo -n setcap cap_net_raw+ep capped_true 2>/dev/null || + unshare -Ur setcap cap_net_raw+ep capped_true 2>/dev/null || + { rm -f capped_true; exit 1; } diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index a84f22ffd..e28ac1530 100644 --- a/coregrind/m_libcfile.c +++ b/coregrind/m_libcfile.c @@ -38,6 +38,7 @@ #include "pub_core_libcproc.h" // VG_(getpid), VG_(getppid) #include "pub_core_clientstate.h" // VG_(fd_hard_limit) #include "pub_core_mallocfree.h" // VG_(realloc) +#include "pub_core_options.h" // VG_(should_we_allow_file_caps) #include "pub_core_syscall.h" /* IMPORTANT: on Darwin it is essential to use the _nocancel versions @@ -1068,7 +1069,9 @@ Int VG_(access) ( const HChar* path, Bool irusr, Bool iwusr, Bool ixusr ) thinks it does). However, the caller may indicate that setuid executables are allowed, for example if we are going to exec them but not trace into them (iow, client sys_execve when - clo_trace_children == False). + clo_trace_children == False). Executables carrying file + capabilities are also run when they match --allow-file-caps=; they + then run without those capabilities. If VKI_EACCES is returned (iow, permission was refused), then *is_setuid is set to True iff permission was refused because the @@ -1099,7 +1102,8 @@ Int VG_(check_executable)(/*OUT*/Bool* is_setuid, } res = VG_(getxattr)(f, "security.capability", (Addr)0, 0); - if (!sr_isError(res) && !allow_setuid) { + if (!sr_isError(res) && !allow_setuid + && !VG_(should_we_allow_file_caps)(f)) { if (is_setuid) *is_setuid = True; return VKI_EACCES; diff --git a/coregrind/m_main.c b/coregrind/m_main.c index e1bfae2b5..c7df35e5c 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -104,6 +104,9 @@ static void usage_NORETURN ( int need_help ) " --trace-children-skip-by-arg=patt1,patt2,... same as --trace-children-skip=\n" " but check the argv[] entries for children, rather\n" " than the exe name, to make a follow/no-follow decision\n" +" --allow-file-caps=patt1,patt2,... specifies a list of executables\n" +" to run despite their file capabilities, which\n" +" they do not get under Valgrind\n" " --child-silent-after-fork=no|yes omit child output between fork & exec? [no]\n" " --vgdb=no|yes|full activate gdbserver? [yes]\n" " full is slower but provides precise watchpoint/step\n" @@ -505,6 +508,12 @@ static void process_option (Clo_Mode mode, // here. else if VG_STR_CLOM(cloE, arg, "--tool", VG_(clo_toolname)) {} + // Set up VG_(clo_allow_file_caps). This is needed by + // VG_(ii_create_image), which checks the client executable + // before main_process_cmd_line_options(). + else if VG_STR_CLOM(cloE, arg, "--allow-file-caps", + VG_(clo_allow_file_caps)) {} + // Set up VG_(clo_max_stackframe) and VG_(clo_main_stacksize). // These are needed by VG_(ii_create_image), which happens // before main_process_cmd_line_options(). diff --git a/coregrind/m_options.c b/coregrind/m_options.c index 612ef8d8b..d344db9fb 100644 --- a/coregrind/m_options.c +++ b/coregrind/m_options.c @@ -123,6 +123,7 @@ const HChar* VG_(clo_soname_synonyms) = NULL; Bool VG_(clo_trace_children) = False; const HChar* VG_(clo_trace_children_skip) = NULL; const HChar* VG_(clo_trace_children_skip_by_arg) = NULL; +const HChar* VG_(clo_allow_file_caps) = NULL; Bool VG_(clo_child_silent_after_fork) = False; const HChar *VG_(clo_log_fname_unexpanded) = NULL; const HChar *VG_(clo_xml_fname_unexpanded) = NULL; @@ -424,6 +425,30 @@ static HChar const* consume_field ( HChar const* c ) { return c; } +Bool VG_(should_we_allow_file_caps) ( const HChar* exe_name ) +{ + HChar const* last = VG_(clo_allow_file_caps); + + if (last == NULL || exe_name == NULL) + return False; + + while (*last) { + Bool matches; + HChar* patt; + HChar const* first = consume_commas(last); + last = consume_field(first); + if (first == last) + break; + patt = VG_(calloc)("m_options.swafc.1", last - first + 1, 1); + VG_(memcpy)(patt, first, last - first); + matches = VG_(string_match)(patt, exe_name); + VG_(free)(patt); + if (matches) + return True; + } + return False; +} + /* Should we trace into this child executable (across execve, spawn etc) ? This involves considering --trace-children=, --trace-children-skip=, --trace-children-skip-by-arg=, and the name diff --git a/coregrind/pub_core_options.h b/coregrind/pub_core_options.h index 6462ae90c..945a96c27 100644 --- a/coregrind/pub_core_options.h +++ b/coregrind/pub_core_options.h @@ -124,6 +124,10 @@ extern const HChar* VG_(clo_trace_children_skip); tested against the arguments for child processes, rather than the executable name. */ extern const HChar* VG_(clo_trace_children_skip_by_arg); +/* String containing comma-separated patterns for executable names + that may be run although they carry file capabilities. They run + without those capabilities, since Valgrind loads them itself. */ +extern const HChar* VG_(clo_allow_file_caps); /* After a fork, the child's output can become confusingly intermingled with the parent's output. This is especially problematic when VG_(clo_xml) is True. Setting @@ -393,6 +397,9 @@ extern Bool VG_(clo_dsymutil); /* Outputs the list of dynamically changeable options. */ extern void VG_(list_dynamic_options) (void); +/* Does the executable name match a pattern of --allow-file-caps= ? */ +extern Bool VG_(should_we_allow_file_caps) ( const HChar* exe_name ); + /* Should we trace into this child executable (across execve etc) ? This involves considering --trace-children=, --trace-children-skip=, --trace-children-skip-by-arg=, and the name diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 9ab09b51b..ea5dee5cc 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -774,6 +774,25 @@ in most cases. We group the available options by rough categories. + + + + + + Valgrind refuses to run an executable that carries file + capabilities (set with setcap), + as it does for setuid and setgid ones: Valgrind loads the + executable itself, so the kernel never grants it those + capabilities. This option takes a comma separated list of + patterns for the names of executables that should be run + anyway, without their capabilities. Patterns may include the + metacharacters ? + and *, which have the usual + meaning. Setuid and setgid executables are still + refused. + + + diff --git a/none/tests/cmdline1.stdout.exp b/none/tests/cmdline1.stdout.exp index 5d3ea0569..21486743e 100644 --- a/none/tests/cmdline1.stdout.exp +++ b/none/tests/cmdline1.stdout.exp @@ -19,6 +19,9 @@ usage: valgrind [options] prog-and-args --trace-children-skip-by-arg=patt1,patt2,... same as --trace-children-skip= but check the argv[] entries for children, rather than the exe name, to make a follow/no-follow decision + --allow-file-caps=patt1,patt2,... specifies a list of executables + to run despite their file capabilities, which + they do not get under Valgrind --child-silent-after-fork=no|yes omit child output between fork & exec? [no] --vgdb=no|yes|full activate gdbserver? [yes] full is slower but provides precise watchpoint/step diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index 2ed4fc02e..a465fb2aa 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -19,6 +19,9 @@ usage: valgrind [options] prog-and-args --trace-children-skip-by-arg=patt1,patt2,... same as --trace-children-skip= but check the argv[] entries for children, rather than the exe name, to make a follow/no-follow decision + --allow-file-caps=patt1,patt2,... specifies a list of executables + to run despite their file capabilities, which + they do not get under Valgrind --child-silent-after-fork=no|yes omit child output between fork & exec? [no] --vgdb=no|yes|full activate gdbserver? [yes] full is slower but provides precise watchpoint/step