Subject: VUL-0: systemtap-server code exec References: bnc#574243 Signed-Off-By: Tony Jones Fix remaining portions of this CVE (part2) still present post release-1.1 This second part is also referred to as CVE-2010-0412 Covers cases such as stap-client -B CC=/bin/rm CFLAGS=/ ... where the execution is done by make commit b75067caf1bb416af21473e40c917d953531e9f9 Author: Dave Brolley Date: Mon Jan 18 11:56:13 2010 -0500 Correct client-side quoting issues discovered by fche during the server-side reimplementation. Also add the test cases to the test suite. commit 241443ad36a5a2cacb9e8e6f12f808d304835f2a Author: Dave Brolley Date: Tue Feb 2 08:26:01 2010 -0500 PR 11105: Remaining client-side problems: stap-client: Correct handling of embedded newlines in arguments. server_args.exp: Add additional cases discovered by fche and by fuzzing. commit 4240be911e37b817727ecb33f321f0ea389ede61 Author: Dave Brolley Date: Mon Feb 15 15:01:28 2010 -0500 Don't pass client-only options to the server. Also correct parsing of the --server option. commit d2334a2233f4efd055dab021c603f7c046730a66 Author: Dave Brolley Date: Tue Feb 2 14:08:31 2010 -0500 Compile server logging and robustness. Log certificate location and status when starting server. Additional care in handling arguments in stap-serverd. New test case discovered by fuzzing added and fixed. commit c0d1b5a004b9949bb455b7dbe17b335b7cab9ead Author: Frank Ch. Eigler Date: Fri Feb 12 10:25:43 2010 -0500 PR11105 part 2: tighten constraints on stap-server parameters passed to make * util.h, util.cxx (assert_match_regexp): New function. * main.cxx (main): Constrain -R, -r, -a, -D, -S, -q, -B flags. * stap-serverd (listen): Harden stap-server-connect with ulimit/loop. * testsuite/systemtap.server/{client,server}_args.exp: Revised. commit cc9e5488d82b728e568bca1f8d6094856fc8e641 Author: Frank Ch. Eigler Date: Fri Feb 12 10:39:58 2010 -0500 PR11105 part 2a, fix buggy \\. in -r option regexp --- main.cxx | 24 ++-- stap-client | 151 +++++++++++++++++------------ stap-gen-cert | 43 ++++---- stap-serverd | 40 +++++-- testsuite/systemtap.server/client_args.exp | 28 ++--- testsuite/systemtap.server/hello.stp | 2 testsuite/systemtap.server/server_args.exp | 49 +++++---- util.cxx | 36 ++++++ util.h | 2 9 files changed, 238 insertions(+), 137 deletions(-) --- a/main.cxx +++ b/main.cxx @@ -57,7 +57,7 @@ version () << "SystemTap translator/driver " << "(version " << VERSION << "/" << dwfl_version (NULL) << " " << GIT_MESSAGE << ")" << endl - << "Copyright (C) 2005-2009 Red Hat, Inc. and others" << endl + << "Copyright (C) 2005-2010 Red Hat, Inc. and others" << endl << "This is free software; see the source for copying conditions." << endl; } @@ -670,12 +670,12 @@ main (int argc, char * const argv []) break; case 'o': + // NB: client_options not a problem, since pass 1-4 does not use output_file. s.output_file = string (optarg); break; case 'R': - if (client_options) - client_options_disallowed += client_options_disallowed.empty () ? "-R" : ", -R"; + if (client_options) { cerr << "ERROR: -R invalid with --client-options" << endl; usage(s,1); } s.runtime_path = string (optarg); break; @@ -684,6 +684,7 @@ main (int argc, char * const argv []) client_options_disallowed += client_options_disallowed.empty () ? "-m" : ", -m"; s.module_name = string (optarg); save_module = true; + // XXX: convert to assert_regexp_match() { string::size_type len = s.module_name.length(); @@ -728,15 +729,14 @@ main (int argc, char * const argv []) break; case 'r': - if (client_options) - client_options_disallowed += client_options_disallowed.empty () ? "-r" : ", -r"; + if (client_options) // NB: no paths! + assert_regexp_match("-r parameter from client", optarg, "^[a-z0-9_.-]+$"); setup_kernel_release(s, optarg); break; case 'a': - if (client_options) - client_options_disallowed += client_options_disallowed.empty () ? "-a" : ", -a"; - s.architecture = string(optarg); + assert_regexp_match("-a parameter", optarg, "^[a-z0-9_-]+$"); + s.architecture = string(optarg); break; case 'k': @@ -783,16 +783,19 @@ main (int argc, char * const argv []) break; case 'D': + assert_regexp_match ("-D parameter", optarg, "^[a-z_][a-z_0-9]*(=[a-z_0-9]+)?$"); if (client_options) client_options_disallowed += client_options_disallowed.empty () ? "-D" : ", -D"; s.macros.push_back (string (optarg)); break; case 'S': + assert_regexp_match ("-S parameter", optarg, "^[0-9]+(,[0-9]+)?$"); s.size_option = string (optarg); break; case 'q': + if (client_options) { cerr << "ERROR: -q invalid with --client-options" << endl; usage(s,1); } s.tapset_compile_coverage = true; break; @@ -823,9 +826,8 @@ main (int argc, char * const argv []) break; case 'B': - if (client_options) - client_options_disallowed += client_options_disallowed.empty () ? "-B" : ", -B"; - s.kbuildflags.push_back (string (optarg)); + if (client_options) { cerr << "ERROR: -B invalid with --client-options" << endl; usage(s,1); } + s.kbuildflags.push_back (string (optarg)); break; case 0: --- a/stap-client +++ b/stap-client @@ -2,7 +2,7 @@ # Compile server client for systemtap # -# Copyright (C) 2008, 2009, 2010 Red Hat Inc. +# Copyright (C) 2008-2010 Red Hat Inc. # # This file is part of systemtap, and is free software. You can # redistribute it and/or modify it under the terms of the GNU General @@ -84,34 +84,34 @@ function parse_options { # Each command line argument will be written to its own file within the # request package. argc=1 + packed_options='-' arg_subst= while test $# != 0 do advance=0 dash_seen=0 + client_arg=0 # Start of a new token. first_token="$1" until test $advance != 0 do # Identify the next option - first_char=`expr "$first_token" : '\(.\).*'` + first_char="${first_token:0:1}" second_char= if test $dash_seen = 0; then if test "$first_char" = "-"; then if test "$first_token" != "-"; then # It's not a lone dash, so it's an option. # Is it a long option (i.e. --option)? - second_char=`expr "$first_token" : '.\(.\).*'` + second_char="${first_token:1:1}" if test "X$second_char" = "X-"; then - long_option=`expr "$first_token" : '--\(.*\)=.*'` - test "X$long_option" != "X" || long_option=`expr "$first_token" : '--\(.*\)'` - case $long_option in - ssl) + case "$first_token" in + --ssl=*) process_ssl "$first_token" ;; - server) + --server=*) process_server "$first_token" ;; *) @@ -123,9 +123,9 @@ function parse_options { fi # It's not a lone dash, or a long option, so it's a short option string. # Remove the dash. - first_token=`expr "$first_token" : '-\(.*\)'` + first_token="${first_token:1}" dash_seen=1 - first_char=`expr "$first_token" : '\(.\).*'` + first_char="${first_token:0:1}" fi fi if test $dash_seen = 0; then @@ -145,69 +145,69 @@ function parse_options { # We are at the start of an option. Look at the first character. case $first_char in a) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_a "$stap_arg" ;; B) - get_arg $first_token "$2" + get_arg "$first_token" "$2" ;; c) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_c "$stap_arg" ;; D) - get_arg $first_token "$2" + get_arg "$first_token" "$2" ;; e) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_e "$stap_arg" ;; I) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_I "$stap_arg" ;; k) keep_temps=1 ;; l) - get_arg $first_token "$2" + get_arg "$first_token" "$2" p_phase=2 ;; L) - get_arg $first_token "$2" + get_arg "$first_token" "$2" p_phase=2 ;; m) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_m "$stap_arg" ;; o) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_o "$stap_arg" ;; p) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_p "$stap_arg" ;; r) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_r "$stap_arg" ;; R) - get_arg $first_token "$2" + get_arg "$first_token" "$2" process_R "$stap_arg" ;; s) - get_arg $first_token "$2" + get_arg "$first_token" "$2" ;; S) - get_arg $first_token "$2" + get_arg "$first_token" "$2" ;; v) v_level=$(($v_level + 1)) ;; x) - get_arg $first_token "$2" + get_arg "$first_token" "$2" ;; *) # An unknown or unimportant flag. @@ -216,7 +216,7 @@ function parse_options { if test $advance = 0; then # Just another flag character. Consume it. - first_token=`expr "$first_token" : '.\(.*\)'` + first_token="${first_token:1}" if test "X$first_token" = "X"; then advance=$(($advance + 1)) fi @@ -224,9 +224,19 @@ function parse_options { done # Consume the arguments we just processed. - while test $advance != 0 - do - # Place the argument is a numbered file within our temp + while test $advance != 0; do + # Does the final argument file contain a client-side file + # name which must be changed to a server-side name? + local arg + if test "X$arg_subst" != "X" -a $advance = 1; then + arg="$arg_subst" + arg_subst= + else + arg="$1" + fi + + # If it's not client-only argument, + # place the argument in a numbered file within our temp # directory. # o We don't write a newline at the end, since newline could be # part of the argument. @@ -234,20 +244,16 @@ function parse_options { # in order to avoid having 'echo' interpret the output as # its own option. We then remove the X. # There must be a better way. - echo -n "X$1" > "$tmpdir_client/argv$argc" - sed -i "s|^X||" "$tmpdir_client/argv$argc" - - # Does the final argument file contain client-side data - # which must be changed to server-side data? - if test "X$arg_subst" != "X" -a $advance = 1; then - sed -i "s|$stap_arg|$arg_subst|" "$tmpdir_client/argv$argc" - arg_subst= + if test $client_arg = 0; then + echo -n "X$arg" > "$tmpdir_client/argv$argc" + sed -i "s|^X||" "$tmpdir_client/argv$argc" + argc=$(($argc + 1)) fi # Get the next argument. shift - argc=$(($argc + 1)) advance=$(($advance - 1)) + packed_options='-' done done @@ -256,7 +262,8 @@ function parse_options { if test "X$script_file" != "X"; then local local_name if test "$script_file" != "-"; then - local_name=`generate_client_temp_name "$script_file"` + generate_client_temp_name "$script_file" + local_name="$client_temp_name" else local_name="-" fi @@ -277,8 +284,9 @@ function parse_options { # Collect an argument to the given option function get_arg { # Remove first character. - local opt=`expr "$1" : '\(.\).*'` - local first=`expr "$1" : '.\(.*\)'` + local opt="${1:0:1}" + local first="${1:1}" + packed_options="${packed_options}$opt" # Advance to the next token, if the first one is exhausted. if test "X$first" = "X"; then @@ -294,7 +302,8 @@ function get_arg { # # Process the --ssl option. function process_ssl { - local db=`expr "$1" : '--ssl=\(.*\)'` + client_arg=1 + local db="${1:6}" test "X$db" != "X" || \ fatal "Missing argument to --ssl" @@ -308,7 +317,8 @@ function process_ssl { # # Process the --server option. function process_server { - local spec=`expr "$1" : '--server=\(.*\)'` + client_arg=1 + local spec="${1:9}" test "X$spec" != "X" || \ fatal "Missing argument to --server" @@ -335,12 +345,19 @@ function process_e { fi } -# function: process_I ARGUMENT +# function: process_I ARGUMENT ORIGINAL_ARGUMENT # # Process the -I flag. function process_I { test "X$1" = "X" && return - arg_subst=tapsets/`include_file_or_directory tapsets "$1"` + test "${1:0:1}" = " +" && return + include_file_or_directory tapsets "$1" + if test $advance = 1; then + arg_subst="${packed_options}tapsets/$included_name" + else + arg_subst="tapsets/$included_name" + fi } # function: process_m ARGUMENT @@ -369,7 +386,7 @@ function process_p { # # Process the -r flag. function process_r { - local first_char=`expr "$1" : '\(.\).*'` + local first_char="${1:0:1}" if test "$first_char" = "/"; then # fully specified path kernel_build_tree="$1" @@ -402,23 +419,34 @@ function process_a { fi } -# function: process_R ARGUMENT +# function: process_R ARGUMENT ORIGINAL_ARGUMENT # # Process the -R flag. function process_R { test "X$1" = "X" && return - arg_subst=runtime/`include_file_or_directory runtime "$1"` + test "${1:0:1}" = " +" && return + include_file_or_directory runtime "$1" + if test $advance = 1; then + arg_subst="${packed_options}runtime/$included_name" + else + arg_subst="runtime/$included_name" + fi } # function: include_file_or_directory PREFIX NAME # # Include the given file or directory in the client's temporary -# tree to be sent to the server. +# tree to be sent to the server and save it's name in the variable +# included_name. We use a global variable instread of echoing the +# result since the use of `include_file_or_directory` loses a trailing +# newline. function include_file_or_directory { # Add a symbolic link of the named file or directory to our temporary # directory, but only if the file or directory exists. - local local_name=`generate_client_temp_name "$2"` - echo "$local_name" + generate_client_temp_name "$2" + local local_name="$client_temp_name" + included_name="$local_name" test -e "/$local_name" || return local local_dirname=`dirname "$local_name"` @@ -431,15 +459,20 @@ function include_file_or_directory { # function: generate_client_temp_name NAME # # Generate the name to be used for the given file/directory relative to the -# client's temporary directory. +# client's temporary directory and stores it in the variable +# client_temp_name. We use a global variable instread of echoing the +# result since the use of `generate_client_temp_name` loses a trailing +# newline. function generate_client_temp_name { # Transform the name into a fully qualified path name - local full_name=`echo "X$1" | sed "s,^X\\\([^/]\\\),$wd/\\\\1," | sed 's,^X,,'` + local full_name="$1" + test "${full_name:0:1}" != "/" && full_name="$wd/$full_name" # The same name without the initial / or trailing / - local local_name=`echo "$full_name" | sed 's,^/\(.*\),\1,'` - local_name=`echo "$local_name" | sed 's,\(.*\)/$,\1,'` - echo "$local_name" + local local_name="${full_name:1}" + test "${local_name: -1:1}" = "/" && local_name="${local_name:0:$((${#local_name}-1))}" + + client_temp_name="$local_name" } # function: create_request @@ -456,7 +489,7 @@ function create_request { fatal "Cannot create temporary directory " $tmpdir_client/script cat > "$tmpdir_client/script/$script_file" else - include_file_or_directory script "$script_file" > /dev/null + include_file_or_directory script "$script_file" fi fi @@ -505,7 +538,7 @@ function unpack_response { # 2) a file called stderr # 3) a file called rc # 4) optionally a directory named to match stap?????? - num_files=`ls $tmpdir_server | wc -l` + local num_files=`ls $tmpdir_server | wc -l` test $num_files = 4 -o $num_files = 3 || \ fatal "Wrong number of files in server's temp directory" test -f $tmpdir_server/stdout || \ --- a/stap-gen-cert +++ b/stap-gen-cert @@ -3,7 +3,7 @@ # Generate a certificate for the systemtap server and add it to the # database of trusted servers for the client. # -# Copyright (C) 2008, 2009 Red Hat Inc. +# Copyright (C) 2008-2010 Red Hat Inc. # # This file is part of systemtap, and is free software. You can # redistribute it and/or modify it under the terms of the GNU General @@ -14,61 +14,68 @@ . ${PKGLIBEXECDIR}stap-env # Obtain the certificate database directory name. -serverdb=$1 +serverdb="$1" if test "X$serverdb" = "X"; then - serverdb=$stap_ssl_db/server + serverdb="$stap_ssl_db/server" fi -rm -fr $serverdb +rm -fr "$serverdb" # Create the server's certificate database directory. -if ! mkdir -p -m 755 $serverdb; then +if ! mkdir -p -m 755 "$serverdb"; then echo "Unable to create the server certificate database directory: $serverdb" >&2 exit 1 fi # Create the certificate database password file. Care must be taken # that this file is only readable by the owner. -if ! (touch $serverdb/pw && chmod 600 $serverdb/pw); then +if ! (touch "$serverdb/pw" && chmod 600 "$serverdb/pw"); then echo "Unable to create the server certificate database password file: $serverdb/pw" >&2 exit 1 fi # Generate a random password. -mkpasswd -l 20 > $serverdb/pw 2>/dev/null || \ -apg -a 1 -n 1 -m 20 -x 20 > $serverdb/pw 2>/dev/null || \ -(read -n20 password $serverdb/pw) +mkpasswd -l 20 > "$serverdb/pw" 2>/dev/null || \ +apg -a 1 -n 1 -m 20 -x 20 > "$serverdb/pw" 2>/dev/null || \ +(read -n20 password "$serverdb/pw") # Generate the server certificate database -if ! certutil -N -d $serverdb -f $serverdb/pw > /dev/null; then +if ! certutil -N -d "$serverdb" -f "$serverdb/pw" > /dev/null; then echo "Unable to initialize the server certificate database directory: $serverdb" >&2 exit 1 fi # We need some random noise for generating keys -dd bs=123 count=1 < /dev/urandom > $serverdb/noise 2> /dev/null +dd bs=123 count=1 < /dev/urandom > "$serverdb/noise" 2> /dev/null # Generate a request for the server's certificate. -certutil -R -d $serverdb -f $serverdb/pw -s "CN=Systemtap Compile Server, OU=Systemtap, O=Red Hat, C=US" -o $serverdb/stap.req -z $serverdb/noise 2> /dev/null -rm -fr $serverdb/noise +certutil -R -d "$serverdb" -f "$serverdb/pw" -s "CN=Systemtap Compile Server, OU=Systemtap, O=Red Hat, C=US" \ + -o "$serverdb/stap.req" -z "$serverdb/noise" 2> /dev/null +rm -fr "$serverdb/noise" # Create the certificate file first so that it always has the proper access permissions. -if ! (touch $serverdb/$stap_certfile && chmod 644 $serverdb/$stap_certfile); then +if ! (touch "$serverdb/$stap_certfile" && chmod 644 "$serverdb/$stap_certfile"); then echo "Unable to create the server certificate file: $serverdb/$stap_certfile" >&2 exit 1 fi -# Now generate the actual certificate. -certutil -C -i $serverdb/stap.req -o $serverdb/$stap_certfile -x -d $serverdb -f $serverdb/pw -5 -8 "$HOSTNAME,localhost" >/dev/null <<-EOF +# Now generate the actual certificate. Make is valid for 1 year. +certutil -C -i "$serverdb/stap.req" -o "$serverdb/$stap_certfile" -x -d "$serverdb" \ + -f "$serverdb/pw" -v 12 -5 -8 "$HOSTNAME,localhost" >/dev/null <<-EOF 1 3 7 8 y EOF -rm -fr $serverdb/stap.req +rm -fr "$serverdb/stap.req" # Add the certificate to the server's certificate/key database as a trusted peer, ssl server and object signer -certutil -A -n stap-server -t "PCu,,PCu" -i $serverdb/$stap_certfile -d $serverdb -f $serverdb/pw +certutil -A -n stap-server -t "PCu,,PCu" -i "$serverdb/$stap_certfile" -d "$serverdb" -f "$serverdb/pw" +# Print some information about the certificate. echo "Certificate $serverdb/$stap_certfile created and added to database $serverdb" +certutil -L -d "$serverdb" -n stap-server | \ + awk '/Validity|Not After|Not Before/ { print $0 }' | \ + sed 's/^ */ /' + exit 0 --- a/stap-serverd +++ b/stap-serverd @@ -70,12 +70,18 @@ function initialization { -x `which ${stap_exec_prefix}stap-client 2>/dev/null`; then ${stap_exec_prefix}stap-authorize-server-cert $ssl_db/$stap_certfile >> $logfile 2>&1 fi - elif ! test -f $stap_ssl_db/client/cert8.db; then - # If the client's database does not exist, then initialize it with our certificate. - # Do this only if the client has been installed. - if test -f `which ${stap_exec_prefix}stap-client` -a \ - -x `which ${stap_exec_prefix}stap-client`; then - ${stap_exec_prefix}stap-authorize-server-cert $ssl_db/$stap_certfile >> $logfile 2>&1 + else + echo "Certificate found in database $ssl_db" >> $logfile + certutil -L -d "$ssl_db" -n stap-server | \ + awk '/Validity|Not After|Not Before/ { print $0 }' | \ + sed 's/^ */ /' >> $logfile + if ! test -f $stap_ssl_db/client/cert8.db; then + # If the client's database does not exist, then initialize it with our certificate. + # Do this only if the client has been installed. + if test -f `which ${stap_exec_prefix}stap-client 2>/dev/null` -a \ + -x `which ${stap_exec_prefix}stap-client 2>/dev/null`; then + ${stap_exec_prefix}stap-authorize-server-cert $ssl_db/$stap_certfile >> $logfile 2>&1 + fi fi fi fi @@ -343,11 +349,19 @@ function advertise_presence { function listen { # The stap-server-connect program will listen forever # accepting requests. - ${stap_pkglibexecdir}stap-server-connect \ - -p $port -n $nss_cert -d $ssl_db -w $nss_pw \ - -s "$stap_options" \ - >> $logfile 2>&1 & - wait '%${stap_pkglibexecdir}stap-server-connect' >> $logfile 2>&1 + # CVE-2009-4273 ... or at least, until resource limits fire + while true; do # NB: loop to avoid DoS by deliberate rlimit-induced halt + # NB: impose resource limits in case of mischevious data inducing + # too much / long computation + (ulimit -f 50000 -s 1000 -t 60 -u 20 -v 500000; + exec ${stap_pkglibexecdir}stap-server-connect \ + -p $port -n $nss_cert -d $ssl_db -w $nss_pw \ + -s "$stap_options") & + stap_server_connect_pid=$! + wait + # NB: avoid superfast spinning in case of a ulimit or other failure + sleep 1 + done >> $logfile 2>&1 } # function: warning [ MESSAGE ] @@ -379,8 +393,8 @@ function terminate { wait '%avahi-publish-service' >> $logfile 2>&1 # Kill any running 'stap-server-connect' job. - kill -s SIGTERM '%${stap_pkglibexecdir}stap-server-connect' >> $logfile 2>&1 - wait '%${stap_pkglibexecdir}stap-server-connect' >> $logfile 2>&1 + kill -s SIGTERM $stap_server_connect_pid >> $logfile 2>&1 + wait $stap_server_connect_pid >> $logfile 2>&1 exit } --- a/testsuite/systemtap.server/client_args.exp +++ b/testsuite/systemtap.server/client_args.exp @@ -5,33 +5,34 @@ set test "Invalid Server Client Argument set test_file $srcdir/systemtap.server/test.stp # Test invalid combinations. -set error_regexp ".*You can't specify .* when --unprivileged is specified.*" +set error_regexp ".*(ERROR)|(You can't specify .* when --unprivileged is specified).*" set invalid_options [list \ - "--unprivileged --client-options -a i386" \ "--unprivileged --client-options -B X=Y" \ "--unprivileged --client-options -D X=Y" \ "--unprivileged --client-options -I /tmp" \ "--unprivileged --client-options -m test" \ "--unprivileged --client-options -R /tmp" \ - "--unprivileged --client-options -r [exec uname -r]" \ - "--unprivileged --client-options -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ - "--client-options --unprivileged -a i386" \ + "--unprivileged --client-options -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ "--client-options --unprivileged -B X=Y" \ "--client-options --unprivileged -D X=Y" \ "--client-options --unprivileged -I /tmp" \ "--client-options --unprivileged -m test" \ "--client-options --unprivileged -R /tmp" \ - "--client-options --unprivileged -r [exec uname -r]" \ - "--client-options --unprivileged -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ - "--client-options -a i386 --unprivileged" \ + "--client-options --unprivileged -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ "--client-options -B X=Y --unprivileged" \ "--client-options -D X=Y --unprivileged" \ "--client-options -I /tmp --unprivileged" \ "--client-options -m test --unprivileged" \ "--client-options -R /tmp --unprivileged" \ - "--client-options -r [exec uname -r] --unprivileged" \ - "--client-options -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --unprivileged" \ + "--client-options -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r] --unprivileged" \ + "--client-options -R /path" \ + "-D \"foo;bar\"" \ + "-D 2=4" \ + "-D \"foo;bar\"" \ + "--client-options -r /path" \ + "-S /path" \ + "--client-options -q" \ ] foreach options $invalid_options { @@ -66,9 +67,8 @@ set valid_options [list \ "-D X=Y" \ "-I /tmp" \ "-m test" \ - "-R /tmp" \ "-r [exec uname -r]" \ - "-a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ + "-a i386 -B X=Y -D X=Y -I /tmp -m test -r [exec uname -r]" \ "--unprivileged" \ "--unprivileged -a i386" \ "--unprivileged -B X=Y" \ @@ -80,13 +80,11 @@ set valid_options [list \ "--unprivileged -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ "--client-options" \ "--client-options -a i386" \ - "--client-options -B X=Y" \ "--client-options -D X=Y" \ "--client-options -I /tmp" \ "--client-options -m test" \ - "--client-options -R /tmp" \ "--client-options -r [exec uname -r]" \ - "--client-options -a i386 -B X=Y -D X=Y -I /tmp -m test -R /tmp -r [exec uname -r]" \ + "--client-options -a i386 -D X=Y -I /tmp -m test -r [exec uname -r]" \ "--unprivileged --client-options" \ "--client-options --unprivileged" \ "--unprivileged -a i386 --client-options" \ --- a/testsuite/systemtap.server/hello.stp +++ b/testsuite/systemtap.server/hello.stp @@ -1,4 +1,4 @@ -#! stap +#! stap -p5 probe begin { --- a/testsuite/systemtap.server/server_args.exp +++ b/testsuite/systemtap.server/server_args.exp @@ -40,11 +40,13 @@ proc stap_direct_and_with_client {stap s # Some messages contain the names of files or directories # and will be prefixed for the client. if {[regexp "^ (.*)" $expected_line match data]} { - if {[regexp "^ tapsets/.*/$data" $line]} { + # Special characters in the regexp need to be quoted. + regsub -all "\[\"\\\\;\*\]" $data {\\\0} data + if {[regexp "^ tapsets.*/$data" $line]} { incr n continue } - if {[regexp "^ runtime/.*/$data" $line]} { + if {[regexp "^ runtime.*/$data" $line]} { incr n continue } @@ -57,7 +59,9 @@ proc stap_direct_and_with_client {stap s } } else { if {[regexp "^Input file '(.*)' is empty or missing." $expected_line match data]} { - if {[regexp "^Input file 'script/.*/$data' is empty or missing." $line]} { + # Special characters in the regexp need to be quoted. + regsub -all "\[\"\\\\;\*\]" $data {\\\0} data + if {[regexp "^Input file 'script.*/$data' is empty or missing." $line]} { incr n continue } @@ -110,21 +114,27 @@ if {[installtest_p]} then { # for debugging a currently failing case and helps to ensure that previously # fixed cases do not regress. set previously_fixed [list \ + "-p1 -I=\\w94\nbh -e -Dhfuo0iu7 -c" \ + "-p1 -I8o\\2ie -e'1\\ -D\n\" -c" \ + "-p1 -Ira\\3;c g -e0fle'qq -Dr/316k\\o8 -cjyoc\n3" \ + "-p1 -I6p3 -elc -Dqgsgv' -c" \ + "-p1 -I\"vyv;z -ej\"/3 -D/ 01qck\n -c3u55zut" \ + "-p1 -I1 -eo9e\nx047q -D9xyefk0a -cvl98/x1'i" \ "-p1 -c; test.stp" \ - "-p1 -I4hgy96 -R -e5oo39p -Bile\\vp -Ddx8v -c4;" \ - "-p1 -I -Repwd9 -esq3wors -Btmk;\\t -Dz -c*eibz8h2e" \ - "-p1 -I -Ry a -em339db5 -B;ae41428d -Du2;c0ps -ch9o\\" \ - "-p1 -Ipfjps4 -Rx479 -ebug4dc -Bih;fe2 -Du8vd fvkl -c" \ - "-p1 -I0\"nspzjyf -R -e5r3up8h -Bgqnyjq6w -Dmi;ojp9m -cx;a2fat" \ - "-p1 -Iu -R9 -ek7;r -Big -Dcu\"; -c\"hc" \ - "-p1 -Icd4fidq -Rkj m40mv -edn -B7ria -D;8ha\\cjr -c1*vnq" \ - "-p1 -I;3 -R3lq;vp -er8e -Bgdqjqdy -D -cb6k29z" \ - "-p1 -Ircj -R -e -B -D -c\\vmww" \ - "-p1 -Illc5 -Rug*\\o -e65wof9 -B qr*=x7x5 -D -cgx;" \ - "-p1 -Iyaj420=3 -R -e\" -Bx68j -D -cd'5mi" \ - "-p1 -Ir -Rwd8;;sjl -e -Bxh; -D29\\ -cj2szt;4" \ - "-p1 -Ibno3=b4sk -R*5 -e' -Byl63flos -Dg2-j;e -c2ijx'" \ - "-p1 -I285v7pl -R9a -eo5\\0 -Bfs* -D86s -c-c*v" \ + "-p1 -I4hgy96 -e5oo39p -Ddx8v -c4;" \ + "-p1 -I -esq3wors -Dz -c*eibz8h2e" \ + "-p1 -I a -em339db5 -Du2;c0ps -ch9o\\" \ + "-p1 -Ipfjps4 -ebug4dc -Du8vd fvkl -c" \ + "-p1 -I0\"nspzjyf -e5r3up8h -Dmi;ojp9m -cx;a2fat" \ + "-p1 -Iu -ek7;r -Dcu\"; -c\"hc" \ + "-p1 -Icd4fidq m40mv -edn -D;8ha\\cjr -c1*vnq" \ + "-p1 -I;3 -er8e -D -cb6k29z" \ + "-p1 -Ircj -e -D -c\\vmww" \ + "-p1 -Illc5 -e65wof9 qr*=x7x5 -D -cgx;" \ + "-p1 -Iyaj420=3 -e\" -D -cd'5mi" \ + "-p1 -Ir -e -D29\\ -cj2szt;4" \ + "-p1 -Ibno3=b4sk -e' -Dg2-j;e -c2ijx'" \ + "-p1 -I285v7pl -eo5\\0 -D86s -c-c*v" \ ] set i 0 @@ -140,7 +150,10 @@ foreach options $previously_fixed { # Generate semi-random arguments containing with potential problem characters. # Check that running systemtap with the client/server generates output # comparable to running stap directly. -set dangerous_options [list "-I" "-R" "-e" "-B" "-D" "-c"] +set dangerous_options [list "-I" "-e" "-D" "-c" "-S"] +# NB: Other options could be candidates here, like -r and -B, but +# there stap-server imposes more restrictions than local stap, so +# this simple error-matching test cannot use them. set argchars "0123456789/;*'=-\\\"\n abcdefghijklmnopqrstuvwxyz" for {set i 0} {$i < $iterations} {incr i} { --- a/util.cxx +++ b/util.cxx @@ -1,5 +1,5 @@ // Copyright (C) Andrew Tridgell 2002 (original file) -// Copyright (C) 2006, 2009 Red Hat Inc. (systemtap changes) +// Copyright (C) 2006-2010 Red Hat Inc. (systemtap changes) // // This program is free software; you can redistribute it and/or // modify it under the terms of the GNU General Public License as @@ -19,6 +19,8 @@ #include "sys/sdt.h" #include #include +#include +#include extern "C" { #include @@ -31,6 +33,7 @@ extern "C" { #include #include #include +#include } using namespace std; @@ -413,4 +416,35 @@ kill_stap_spawn(int sig) return spawned_pid ? kill(spawned_pid, sig) : 0; } + +void assert_regexp_match (const string& name, const string& value, const string& re) +{ + typedef map cache; + static cache compiled; + cache::iterator it = compiled.find (re); + regex_t* r = 0; + if (it == compiled.end()) + { + r = new regex_t; + int rc = regcomp (r, re.c_str(), REG_ICASE|REG_NOSUB|REG_EXTENDED); + if (rc) { + cerr << "regcomp " << re << " (" << name << ") error rc=" << rc << endl; + exit(1); + } + compiled[re] = r; + } + else + r = it->second; + + // run regexec + int rc = regexec (r, value.c_str(), 0, 0, 0); + if (rc) + { + cerr << "ERROR: Safety pattern mismatch for " << name + << " ('" << value << "' vs. '" << re << "') rc=" << rc << endl; + exit(1); + } +} + + /* vim: set sw=2 ts=8 cino=>4,n-2,{2,^-2,t0,(0,u0,w1,M1 : */ --- a/util.h +++ b/util.h @@ -21,7 +21,7 @@ const std::string cmdstr_quoted(const st std::string git_revision(const std::string& path); int stap_system(int verbose, const std::string& command); int kill_stap_spawn(int sig); - +void assert_regexp_match (const std::string& name, const std::string& value, const std::string& re); // stringification generics