From 19d91f6c2fcc4dea3b86e05721275cf0f8117e74 Mon Sep 17 00:00:00 2001
From: Josh Stone <jistone@redhat.com>
Date: Thu, 14 Jan 2010 16:40:45 -0800
Subject: PR11151: Recover stap_uprobes slots on process exit

When a process exits, it won't necessarily bother to munmap all of its
shared libraries.  This patch makes sure that with uprobes in libraries,
we still grab an exit notification and clear everything out.

* runtime/uprobes-common.c (stap_uprobe_process_munmap): New.
* tapsets.cxx (uprobe_derived_probe_group::emit_module_decls): Use above
  callback so that we can recover our resources on process exit.
---
 runtime/uprobes-common.c | 15 +++++++++++++++
 runtime/uprobes-common.h |  1 +
 tapsets.cxx              |  1 +
 3 files changed, 17 insertions(+)

diff --git a/runtime/uprobes-common.c b/runtime/uprobes-common.c
index b0273ba4..58e3a05f 100644
--- a/runtime/uprobes-common.c
+++ b/runtime/uprobes-common.c
@@ -286,4 +286,19 @@ static int stap_uprobe_munmap_found (struct stap_task_finder_target *tgt, struct
   return stap_uprobe_change_minus (tsk, addr, length, stf);
 }
 
+/* The task_finder_callback we use for ET_DYN targets.
+   This just forces an unmap of everything as the process exits.
+   (PR11151) */
+static int stap_uprobe_process_munmap (struct stap_task_finder_target *tgt, struct task_struct *tsk, int register_p, int process_p) {
+  const struct stap_uprobe_tf *stf = container_of(tgt, struct stap_uprobe_tf, finder);
+  if (! process_p) return 0; /* ignore threads */
+  #ifdef DEBUG_TASK_FINDER_VMA
+  _stp_dbug (__FUNCTION__,__LINE__, "%cproc pid %d stf %p %p path %s\n", register_p?'+':'-', tsk->tgid, tgt, stf, stf->pathname);
+  #endif
+  /* Covering 0->TASK_SIZE means "unmap everything" */
+  if (!register_p)
+    return stap_uprobe_change_minus (tsk, 0, TASK_SIZE, stf);
+  return 0;
+}
+
 #endif /* _UPROBE_COMMON_C_ */
diff --git a/runtime/uprobes-common.h b/runtime/uprobes-common.h
index 68741f4d..990b473a 100644
--- a/runtime/uprobes-common.h
+++ b/runtime/uprobes-common.h
@@ -33,5 +33,6 @@ struct stap_uprobe_spec {
 static int stap_uprobe_process_found (struct stap_task_finder_target *tgt, struct task_struct *tsk, int register_p, int process_p);
 static int stap_uprobe_mmap_found (struct stap_task_finder_target *tgt, struct task_struct *tsk, char *path, unsigned long addr, unsigned long length, unsigned long offset, unsigned long vm_flags);
 static int stap_uprobe_munmap_found (struct stap_task_finder_target *tgt, struct task_struct *tsk, unsigned long addr, unsigned long length);
+static int stap_uprobe_process_munmap (struct stap_task_finder_target *tgt, struct task_struct *tsk, int register_p, int process_p);
 
 #endif /* _UPROBE_COMMON_H_ */
diff --git a/tapsets.cxx b/tapsets.cxx
index 7835b39a..071f92db 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -4632,6 +4632,7 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s)
 		s.op->line() << " .procname=\"" << p->path << "\", ";
               s.op->line() << " .mmap_callback=&stap_uprobe_mmap_found, ";
               s.op->line() << " .munmap_callback=&stap_uprobe_munmap_found, ";
+              s.op->line() << " .callback=&stap_uprobe_process_munmap,";
             }
 
           s.op->line() << " },";
-- 
cgit 


From 80faaf7d57e005f4c2a6864525d2612621b2fbb7 Mon Sep 17 00:00:00 2001
From: Wenji Huang <wenji.huang@oracle.com>
Date: Fri, 15 Jan 2010 14:00:49 +0800
Subject: Improve stack printing format

* runtime/stack.c (_stp_stack_print_tsk): Use _stp_symbol_print.
---
 runtime/stack.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/runtime/stack.c b/runtime/stack.c
index 7dfeb76a..0e537a8e 100644
--- a/runtime/stack.c
+++ b/runtime/stack.c
@@ -212,7 +212,8 @@ void _stp_stack_print_tsk(struct task_struct *tsk, int verbose, int levels)
         for (i = 0; i < maxLevels; ++i) {
                 if (backtrace[i] == 0 || backtrace[i] == ULONG_MAX)
                         break;
-                _stp_printf("%lx ", backtrace[i]);
+		_stp_symbol_print(backtrace[i]);
+		_stp_print_char('\n');
         }
 #endif
 }
-- 
cgit 


From e4d80588594a7495a3efedbd3a4281df13ff253b Mon Sep 17 00:00:00 2001
From: Dave Brolley <brolley@redhat.com>
Date: Fri, 15 Jan 2010 00:47:32 -0500
Subject: PR11105: stap-client wire protocol change

---
 stap-client | 213 ++++++++++++++++++++++++++++--------------------------------
 1 file changed, 99 insertions(+), 114 deletions(-)

diff --git a/stap-client b/stap-client
index 869bff42..fc3d9908 100755
--- a/stap-client
+++ b/stap-client
@@ -81,18 +81,19 @@ function initialization {
 # output.
 #
 function parse_options {
-    cmdline=
-    cmdline1=
-    cmdline2=
+    # Each command line argument will be written to its own file within the
+    # request package.
+    argc=1
+    arg_subst=
 
     while test $# != 0
     do
-	advance_p=0
+	advance=0
 	dash_seen=0
 
         # Start of a new token.
-	first_token=$1
-	until test $advance_p != 0
+	first_token="$1"
+	until test $advance != 0
 	do
             # Identify the next option
 	    first_char=`expr "$first_token" : '\(.\).*'`
@@ -108,18 +109,16 @@ function parse_options {
 			    test "X$long_option" != "X" || long_option=`expr "$first_token" : '--\(.*\)'`
 			    case $long_option in
 				ssl)
-				    process_ssl $first_token
+				    process_ssl "$first_token"
 				    ;;
 				server)
-				    process_server $first_token
+				    process_server "$first_token"
 				    ;;
 				*)
-		                    # An unknown or unimportant option.
-				    # Ignore it, but pass it on to the server.
-				    cmdline2="$cmdline2 $first_token"
+		                    # An unknown or unimportant option. Ignore it.
 				    ;;
 			    esac
-			    advance_p=$(($advance_p + 1))
+			    advance=$(($advance + 1))
 			    break
 			fi
 	                # It's not a lone dash, or a long option, so it's a short option string.
@@ -136,14 +135,9 @@ function parse_options {
 	            # then it could be the name of the script file.
 		    if test "X$e_script" = "X" -a "X$script_file" = "X"; then
 			script_file="$first_token"
-			cmdline1="$cmdline2"
-			cmdline2=
-		    elif test "$first_char" != "'"; then
-			cmdline2="$cmdline2 '$first_token'"
-		    else
-			cmdline2="$cmdline2 $first_token"
+			script_file_argc=$argc
 		    fi
-		    advance_p=$(($advance_p + 1))
+		    advance=$(($advance + 1))
 		    break
 		fi
 	    fi
@@ -151,112 +145,122 @@ 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
-		    process_a $stap_arg
+		    get_arg $first_token "$2"
+		    process_a "$stap_arg"
 		    ;;	
 		B)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    ;;	
 		c)
 		    get_arg $first_token "$2"
 		    process_c "$stap_arg"
 		    ;;
 		D)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    ;;
 		e)
 		    get_arg $first_token "$2"
 		    process_e "$stap_arg"
 		    ;;
 		I)
-		    get_arg $first_token $2
-		    process_I $stap_arg
+		    get_arg $first_token "$2"
+		    process_I "$stap_arg"
 		    ;;	
 		k)
 		    keep_temps=1
 		    ;;
 		l)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    p_phase=2
 		    ;;
 		L)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    p_phase=2
 		    ;;
 		m)
-		    get_arg $first_token $2
-		    process_m $stap_arg
+		    get_arg $first_token "$2"
+		    process_m "$stap_arg"
 		    ;;
 		o)
-		    get_arg $first_token $2
-		    process_o $stap_arg
+		    get_arg $first_token "$2"
+		    process_o "$stap_arg"
 		    ;;
 		p)
-		    get_arg $first_token $2
-		    process_p $stap_arg
+		    get_arg $first_token "$2"
+		    process_p "$stap_arg"
 		    ;;
 		r)
-		    get_arg $first_token $2
-		    process_r $stap_arg
+		    get_arg $first_token "$2"
+		    process_r "$stap_arg"
 		    ;;	
 		R)
-		    get_arg $first_token $2
-		    process_R $stap_arg
+		    get_arg $first_token "$2"
+		    process_R "$stap_arg"
 		    ;;	
 		s)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    ;;	
 		S)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    ;;	
 		v)
 		    v_level=$(($v_level + 1))
 		    ;;
 		x)
-		    get_arg $first_token $2
-		    cmdline2="${cmdline2} -$first_char '$stap_arg'"
+		    get_arg $first_token "$2"
 		    ;;
 		*)
-		    # An unknown or unimportant flag. Ignore it, but pass it on to the server.
+		    # An unknown or unimportant flag.
 		    ;;
 	    esac
 
-	    if test $advance_p = 0; then
+	    if test $advance = 0; then
 	        # Just another flag character. Consume it.
-		cmdline2="$cmdline2 -$first_char"
 		first_token=`expr "$first_token" : '.\(.*\)'`
 		if test "X$first_token" = "X"; then
-		    advance_p=$(($advance_p + 1))
+		    advance=$(($advance + 1))
 		fi
 	    fi
 	done
 
         # Consume the arguments we just processed.
-	while test $advance_p != 0
+	while test $advance != 0
 	do
+	    # Place the argument is 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.
+	    # o We add an X to the beginning of the file
+	    #   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=
+	    fi
+
+	    # Get the next argument.
 	    shift
-	    advance_p=$(($advance_p - 1))
+	    argc=$(($argc + 1))
+	    advance=$(($advance - 1))
 	done
     done
 
     # If the script file was given and it's not '-', then replace it with its
-    # client-temp-name in the command string.
+    # client-temp-name in its argument file.
     if test "X$script_file" != "X"; then
 	local local_name
 	if test "$script_file" != "-"; then
 	    local_name=`generate_client_temp_name "$script_file"`
 	else
-	    local_name="$script_file"
+	    local_name="-"
 	fi
-	cmdline="$cmdline1 'script/$local_name' $cmdline2"
-    else
-	cmdline="$cmdline2"
+	echo "script/$local_name" > "$tmpdir_client/argv$script_file_argc"
     fi
 
     # Processing based on final options settings
@@ -266,12 +270,6 @@ function parse_options {
     # We must have at least one usable certificate database.
     test "X$local_ssl_dbs" != "X " -o "X$public_ssl_dbs" != "X" || \
 	fatal "No usable certificate databases found"
-
-    # We can use any server if the phase is less than 4
-    # But don't for now....
-    #if test $p_phase -lt 4; then
-    #	find_all="--all"
-    #fi
 }
 
 # function: get_arg FIRSTWORD SECONDWORD
@@ -284,13 +282,12 @@ function get_arg {
 
     # Advance to the next token, if the first one is exhausted.
     if test "X$first" = "X"; then
-	shift
-	advance_p=$(($advance_p + 1))
-	first=$1
+	advance=$(($advance + 1))
+	first="$2"
     fi
 
     stap_arg="$first"
-    test "X$first" != "X" && advance_p=$(($advance_p + 1))
+    test "X$first" != "X" && advance=$(($advance + 1))
 }
 
 # function: process_ssl ARGUMENT
@@ -302,7 +299,7 @@ function process_ssl {
     test "X$db" != "X" || \
 	fatal "Missing argument to --ssl"
 
-    check_db $db || return
+    check_db "$db" || return
 
     additional_local_ssl_dbs="$additional_local_ssl_dbs $db"
 }
@@ -324,8 +321,6 @@ function process_server {
 # Process the -c flag.
 function process_c {
     c_cmd="$1"
-    cmdline2="${cmdline2} -c"
-    test "X$c_cmd" != "X" && cmdline2="${cmdline2} '$c_cmd'"
 }
 
 # function: process_e ARGUMENT
@@ -336,22 +331,16 @@ function process_e {
     # which may have already been identified.
     if test "X$e_script" = "X"; then
 	e_script="$1"
-	if test "X$script_file" != "X"; then
-	    cmdline2="$cmdline1 '$script_file' $cmdline2"
-	    cmdline1=
-	    script_file=
-	fi
+	script_file=
     fi
-    cmdline2="${cmdline2} -e '$1'"
 }
 
 # function: process_I ARGUMENT
 #
 # Process the -I flag.
 function process_I {
-    local local_name=`include_file_or_directory tapsets $1`
-    test "X$local_name" != "X" || return
-    cmdline2="${cmdline2} -I 'tapsets/$local_name'"
+    test "X$1" = "X" && return
+    arg_subst=tapsets/`include_file_or_directory tapsets "$1"`
 }
 
 # function: process_m ARGUMENT
@@ -360,7 +349,6 @@ function process_I {
 function process_m {
     module_name="$1"
     m_name="$1"
-    cmdline2="${cmdline2} -m '$1'"
 }
 
 # function: process_o ARGUMENT
@@ -368,15 +356,13 @@ function process_m {
 # Process the -o flag.
 function process_o {
     stdout_redirection="$1"
-    cmdline2="${cmdline2} -o '$1'"
 }
 
 # function: process_p ARGUMENT
 #
 # Process the -p flag.
 function process_p {
-    p_phase=$1
-    cmdline2="${cmdline2} -p '$1'"
+    p_phase="$1"
 }
 
 # function: process_r ARGUMENT
@@ -386,22 +372,22 @@ function process_r {
     local first_char=`expr "$1" : '\(.\).*'`
 
     if test "$first_char" = "/"; then # fully specified path
-        kernel_build_tree=$1
+        kernel_build_tree="$1"
         version_file_name="$kernel_build_tree/include/config/kernel.release"
         # The file include/config/kernel.release within the
         # build tree is used to pull out the version information
-	release=`cat $version_file_name 2>/dev/null`
+	release=`cat "$version_file_name" 2>/dev/null`
 	if test "X$release" = "X"; then
 	    fatal "Missing $version_file_name"
 	    return
 	fi
     else
 	# kernel release specified directly
-	release=$1
+	release="$1"
     fi
 
     if test "X$release" != "X$uname_r"; then
-	uname_r=$release
+	uname_r="$release"
 	find_all="--all"
     fi
 }
@@ -411,7 +397,7 @@ function process_r {
 # Process the -a flag.
 function process_a {
     if test "X$1" != "X$arch"; then
-	arch=$1
+	arch="$1"
 	find_all="--all"
     fi
 }
@@ -420,9 +406,8 @@ function process_a {
 #
 # Process the -R flag.
 function process_R {
-    local local_name=`include_file_or_directory runtime $1`
-    test "X$local_name" != "X" || return
-    cmdline2="${cmdline2} -R 'runtime/$local_name'"
+    test "X$1" = "X" && return
+    arg_subst=runtime/`include_file_or_directory runtime "$1"`
 }
 
 # function: include_file_or_directory PREFIX NAME
@@ -434,7 +419,7 @@ function include_file_or_directory {
     # directory, but only if the file or directory exists.
     local local_name=`generate_client_temp_name "$2"`
     echo "$local_name"
-    test -e /$local_name || return
+    test -e "/$local_name" || return
 
     local local_dirname=`dirname "$local_name"`
     mkdir -p "$tmpdir_client/$1/$local_dirname" || \
@@ -476,7 +461,6 @@ function create_request {
     fi
 
     # Add the necessary info to special files in our temporary directory.
-    echo "cmdline: $cmdline" > cmdline
     echo "sysinfo: `client_sysinfo`" > sysinfo
 }
 
@@ -499,7 +483,8 @@ function package_request {
     zip_client=$tmpdir_env/`mktemp $tmpdir_client_base.zip.XXXXXX` || \
 	fatal "Cannot create temporary file " $zip_client
 
-    (rm $zip_client && zip -r $zip_client $tmpdir_client_base > /dev/null) || \
+    cd $tmpdir_client
+    (rm -f $zip_client && zip -r $zip_client * > /dev/null) || \
 	fatal "zip of request tree, $tmpdir_client, failed"
 }
 
@@ -515,17 +500,6 @@ function unpack_response {
     unzip -d $tmpdir_server $zip_server > /dev/null || \
 	fatal "Cannot unpack server response, $zip_server"
 
-    # Check the contents of the expanded directory. It should contain a
-    # single directory whose name matches $stap_tmpdir_prefix_server.??????
-    local num_files=`ls $tmpdir_server | wc -l`
-    test $num_files = 1 || \
-	fatal "Wrong number of files in server's temp directory"
-    test -d $tmpdir_server/$stap_tmpdir_prefix_server.?????? || \
-	fatal "`ls $tmpdir_server` does not match the expected name or is not a directory"
-    # Move the contents of the directory down one level.
-    mv $tmpdir_server/$stap_tmpdir_prefix_server.??????/* $tmpdir_server
-    rm -fr $tmpdir_server/$stap_tmpdir_prefix_server.??????
-
     # Check the contents of the directory. It should contain:
     # 1) a file called stdout
     # 2) a file called stderr
@@ -541,7 +515,8 @@ function unpack_response {
     test -f $tmpdir_server/rc || \
 	fatal "`pwd`/$tmpdir_server/rc does not exist or is not a regular file"
 
-    # See if there is a systemtap temp directory
+    # See if there is a systemtap temp directory.  There should be at least an empty one.
+    # ls -l $tmpdir_server
     tmpdir_stap=`cd $tmpdir_server && ls | grep stap......\$ 2>/dev/null`
     if test "X$tmpdir_stap" != "X"; then
 	test -d $tmpdir_server/$tmpdir_stap || \
@@ -564,6 +539,11 @@ function unpack_response {
 	    test $EUID = 0 && chown $EUID:$EUID $tmpdir_stap
 	fi
     fi
+
+    if test $keep_temps = 0; then
+      # Remove the output line due to the synthetic server-side -k
+      sed -i "/^Keeping temporary directory.*/ d" $tmpdir_server/stderr
+    fi
 }
 
 # function: find_and_connect_to_server
@@ -649,16 +629,21 @@ function find_and_connect_to_server {
 	# Remember which ssl certificate database was used to authenticate the chosen
         # server.
 	ssl_db=`${stap_pkglibexecdir}stap-find-servers $find_all | choose_server`
-	test "X$ssl_db" != "X" && return
+	if test "X$ssl_db" != "X"; then
+	    rm -f $tmpdir_client/connect
+	    return
+	fi
 
 	num_servers=`${stap_pkglibexecdir}stap-find-servers $find_all | wc -l`
     fi
 
     if test $num_servers = 0; then
+	rm -f $tmpdir_client/connect
 	fatal "Unable to find a server"
     fi
 
     cat $tmpdir_client/connect >&2
+    rm -f $tmpdir_client/connect
     fatal "Unable to connect to a server"
 }
 
@@ -700,8 +685,8 @@ function choose_server {
 # echo the name of the ssl certificate database used to successfully authenticate
 # the server.
 function send_receive {
-    local server=$1
-    local port=$2
+    local server="$1"
+    local port="$2"
 
     # The server must match the dns name on the certificate
     # and must be 'localhost' if the server is on the local host.
@@ -880,9 +865,9 @@ function staprun_PATH {
 #
 # Check the security of the given database directory.
 function check_db {
-    local dir=$1
-    local euid=$2
-    local user=$3
+    local dir="$1"
+    local euid="$2"
+    local user="$3"
     local rc=0
 
     # Check that we have been given a directory
@@ -957,7 +942,7 @@ function check_db {
 #
 # Check the security of the given database file.
 function check_db_file {
-    local file=$1
+    local file="$1"
     local rc=0
 
     # Check that we have been given a file
-- 
cgit 


From 4b9b5e8597daa65325756b18fcb87b71c60b26aa Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Fri, 15 Jan 2010 03:04:18 -0500
Subject: runtime: better staprun diagnostics for failed signature tests

* modverify.c (verify_module): Print some messages for verbose > 1.
---
 runtime/staprun/modverify.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/runtime/staprun/modverify.c b/runtime/staprun/modverify.c
index a17bb2ec..5d442393 100644
--- a/runtime/staprun/modverify.c
+++ b/runtime/staprun/modverify.c
@@ -272,12 +272,18 @@ int verify_module (const char *signatureName, const char* module_name,
 
   /* Verify the permissions of the certificate database and its files.  */
   if (! check_cert_db_permissions (dbdir))
-    return MODULE_UNTRUSTED;
+    {
+      if (verbose>1) fprintf (stderr, "Certificate db %s permissions too loose\n", dbdir);
+      return MODULE_UNTRUSTED;
+    }
 
   /* Get the size of the signature file.  */
   prStatus = PR_GetFileInfo (signatureName, &info);
   if (prStatus != PR_SUCCESS || info.type != PR_FILE_FILE || info.size < 0)
-    return MODULE_UNTRUSTED; /* Not signed */
+    {
+      if (verbose>1) fprintf (stderr, "Signature file %s not found\n", signatureName);
+      return MODULE_UNTRUSTED; /* Not signed */
+    }
 
   /* Open the signature file.  */
   local_file_fd = PR_Open (signatureName, PR_RDONLY, 0);
-- 
cgit 


From 9e6bf24eef9d73afbccf9d46fa85a27587d6443b Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Fri, 15 Jan 2010 03:05:38 -0500
Subject: server: log controlling pid

* stap-serverd (initialization): Log startup pid.
---
 stap-serverd | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/stap-serverd b/stap-serverd
index f3df884d..4d8a10ce 100755
--- a/stap-serverd
+++ b/stap-serverd
@@ -38,6 +38,8 @@ function initialization {
     # Parse the arguments
     parse_options "$@"
 
+    echo ===== compile server pid $$ started >> $logfile
+
     # What port will we listen on?
     test "X$port" = "X" && port=$((1024+$RANDOM%64000))
     while netstat -atn | awk '{print $4}' | cut -f2 -d: | egrep -q "^$port\$";
-- 
cgit 


From cf4a6df840531c1b30f8cfa7d10981d071911b98 Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Fri, 15 Jan 2010 03:06:52 -0500
Subject: PR11105: robustify stap-server

* main.cxx (main): Always downgrade client-provided -p5 to -p4.
* stap-client (unpack_response): Sanitize stdout due to same.
* stap-server-connect.c: Eliminate a bunch of globals.
  (handle_connection): Make things locals instead.  Base tmp files
  on $TMPDIR.
  (spawn_and_wait): New helper function.
  (handleRequest): New monster function to inline rest of old
  stap-server-request.
---
 main.cxx              |  11 +-
 stap-client           |   5 +
 stap-server-connect.c | 499 ++++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 415 insertions(+), 100 deletions(-)

diff --git a/main.cxx b/main.cxx
index 4a845620..cbedd6e4 100644
--- a/main.cxx
+++ b/main.cxx
@@ -1,5 +1,5 @@
 // systemtap translator/driver
-// Copyright (C) 2005-2009 Red Hat Inc.
+// Copyright (C) 2005-2010 Red Hat Inc.
 // Copyright (C) 2005 IBM Corp.
 // Copyright (C) 2006 Intel Corporation.
 //
@@ -877,6 +877,8 @@ main (int argc, char * const argv [])
 	      break;
 	    case LONG_OPT_UNPRIVILEGED:
 	      s.unprivileged = true;
+              /* NB: for server security, it is essential that once this flag is
+                 set, no future flag be able to unset it. */
 	      break;
 	    case LONG_OPT_CLIENT_OPTIONS:
 	      client_options = true;
@@ -895,6 +897,10 @@ main (int argc, char * const argv [])
 
   // Check for options conflicts.
 
+  if (client_options && s.last_pass > 4)
+    {
+      s.last_pass = 4; /* Quietly downgrade.  Server passed through -p5 naively. */
+    }
   if (client_options && s.unprivileged && ! client_options_disallowed.empty ())
     {
       cerr << "You can't specify " << client_options_disallowed << " when --unprivileged is specified." << endl;
@@ -921,7 +927,6 @@ main (int argc, char * const argv [])
       if (s.kernel_symtab_path == PATH_TBD)
         s.kernel_symtab_path = string("/boot/System.map-") + s.kernel_release;
     }
-
   // Warn in case the target kernel release doesn't match the running one.
   if (s.last_pass > 4 &&
       (string(buf.release) != s.kernel_release ||
@@ -1369,6 +1374,8 @@ pass_5:
   else
     {
       if (s.keep_tmpdir)
+        // NB: the format of this message needs to match the expectations
+        // of stap-server-connect.c.
         clog << "Keeping temporary directory \"" << s.tmpdir << "\"" << endl;
       else
         {
diff --git a/stap-client b/stap-client
index fc3d9908..f8be3838 100755
--- a/stap-client
+++ b/stap-client
@@ -544,6 +544,11 @@ function unpack_response {
       # Remove the output line due to the synthetic server-side -k
       sed -i "/^Keeping temporary directory.*/ d" $tmpdir_server/stderr
     fi
+
+    if test $p_phase = 5; then
+      # Remove the output line due to the synthetic server-side -p4
+      sed -i "/^.*\.ko$/ d" $tmpdir_server/stdout
+    fi
 }
 
 # function: find_and_connect_to_server
diff --git a/stap-server-connect.c b/stap-server-connect.c
index cf0e5a65..bbf5ade7 100644
--- a/stap-server-connect.c
+++ b/stap-server-connect.c
@@ -1,9 +1,9 @@
 /*
   SSL server program listens on a port, accepts client connection, reads
-  the data into a temporary file, calls the systemtap server script and
-  then transmits the resulting fileback to the client.
+  the data into a temporary file, calls the systemtap translator and
+  then transmits the resulting file back to 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 Public
@@ -23,7 +23,16 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <errno.h>
+#include <spawn.h>
+#include <fcntl.h>
+#include <glob.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <wordexp.h>
+#include <sys/param.h>
 
 #include <ssl.h>
 #include <nspr.h>
@@ -31,19 +40,21 @@
 #include <nss.h>
 #include <pk11func.h>
 
+#include "config.h"
 #include "nsscommon.h"
 
-#define READ_BUFFER_SIZE (60 * 1024)
 
 /* Global variables */
 static char             *password = NULL;
 static CERTCertificate  *cert = NULL;
 static SECKEYPrivateKey *privKey = NULL;
 static char             *dbdir   = NULL;
-static char requestFileName[] = "/tmp/stap.server.client.zip.XXXXXX";
-static char responseDirName[] = "/tmp/stap.server.XXXXXX";
-static char responseZipName[] = "/tmp/stap.server.XXXXXX.zip.XXXXXX";
-static const char *stapOptions = "";
+static const char       *stapOptions = "";
+
+
+static PRStatus spawn_and_wait (char **argv,
+                                const char* fd0, const char* fd1, const char* fd2, const char *pwd);
+
 
 static void
 Usage(const char *progName)
@@ -72,24 +83,27 @@ exitErr(char *function)
   exit(1);
 }
 
+
+
+
 /* Function:  readDataFromSocket()
  *
  * Purpose:  Read data from the socket into a temporary file.
  *
  */
-static SECStatus
-readDataFromSocket(PRFileDesc *sslSocket)
+static SECStatus readDataFromSocket(PRFileDesc *sslSocket, const char *requestFileName)
 {
   PRFileDesc *local_file_fd;
   PRFileInfo  info;
   PRInt32     numBytesRead;
   PRInt32     numBytesWritten;
   PRInt32     totalBytes;
+#define READ_BUFFER_SIZE 4096
   char        buffer[READ_BUFFER_SIZE];
 
   /* Open the output file.  */
   local_file_fd = PR_Open(requestFileName, PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE,
-			  PR_IRUSR | PR_IWUSR | PR_IRGRP | PR_IWGRP | PR_IROTH);
+			  PR_IRUSR | PR_IWUSR);
   if (local_file_fd == NULL)
     {
       fprintf (stderr, "could not open output file %s\n", requestFileName);
@@ -97,6 +111,7 @@ readDataFromSocket(PRFileDesc *sslSocket)
     }
 
   /* Read the number of bytes to be received.  */
+  /* XXX: impose a limit to prevent disk space consumption DoS */
   numBytesRead = PR_Read(sslSocket, & info.size, sizeof (info.size));
   if (numBytesRead == 0) /* EOF */
     {
@@ -123,10 +138,11 @@ readDataFromSocket(PRFileDesc *sslSocket)
 
       /* Write to stdout */
       numBytesWritten = PR_Write(local_file_fd, buffer, numBytesRead);
-      if (numBytesWritten < 0)
-	fprintf (stderr, "could not write to output file %s\n", requestFileName);
-      if (numBytesWritten != numBytesRead)
-	fprintf (stderr, "could not write to output file %s\n", requestFileName);
+      if (numBytesWritten < 0 || (numBytesWritten != numBytesRead))
+        {
+          fprintf (stderr, "could not write to output file %s\n", requestFileName);
+          break;
+        }
 #if DEBUG
       fprintf(stderr, "***** Connection read %d bytes.\n", numBytesRead);
 #if 0
@@ -317,27 +333,15 @@ authenticateSocket(PRFileDesc *sslSocket, PRBool requireCert)
  *
  */
 static SECStatus
-writeDataToSocket(PRFileDesc *sslSocket)
+writeDataToSocket(PRFileDesc *sslSocket, const char *responseFileName)
 {
   int         numBytes;
   PRFileDesc *local_file_fd;
-  PRFileInfo  info;
-  PRStatus    prStatus;
 
-  /* Try to open the local file named.	
-   * If successful, then write it to the client.
-   */
-  prStatus = PR_GetFileInfo(responseZipName, &info);
-  if (prStatus != PR_SUCCESS || info.type != PR_FILE_FILE || info.size < 0)
-    {
-      fprintf (stderr, "Input file %s not found\n", responseZipName);
-      return SECFailure;
-    }
-
-  local_file_fd = PR_Open(responseZipName, PR_RDONLY, 0);
+  local_file_fd = PR_Open(responseFileName, PR_RDONLY, 0);
   if (local_file_fd == NULL)
     {
-      fprintf (stderr, "Could not open input file %s\n", responseZipName);
+      fprintf (stderr, "Could not open input file %s\n", responseFileName);
       return SECFailure;
     }
 
@@ -357,7 +361,7 @@ writeDataToSocket(PRFileDesc *sslSocket)
 #if DEBUG
   /* Transmitted bytes successfully. */
   fprintf(stderr, "PR_TransmitFile wrote %d bytes from %s\n",
-	  numBytes, responseZipName);
+	  numBytes, responseFileName);
 #endif
 
   PR_Close(local_file_fd);
@@ -365,10 +369,303 @@ writeDataToSocket(PRFileDesc *sslSocket)
   return SECSuccess;
 }
 
+
+/* Run the translator on the data in the request directory, and produce output
+   in the given output directory. */
+static void handleRequest (const char* requestDirName, const char* responseDirName)
+{
+  char stapstdout[PATH_MAX];
+  char stapstderr[PATH_MAX];
+  char staprc[PATH_MAX];
+#define MAXSTAPARGC 1000 /* sorry, too lazy to dynamically allocate */
+  char* stapargv[MAXSTAPARGC];
+  int stapargc=0;
+  int rc;
+  wordexp_t words;
+  int i;
+  FILE* f;
+  int unprivileged = 0;
+  int stapargv_freestart = 0;
+
+  stapargv[stapargc++]= STAP_PREFIX "/bin/stap";
+
+  /* Transcribe stapOptions.  We use plain wordexp(3), since these
+     options are coming from the local trusted user, so malicious
+     content is not a concern. */
+
+  rc = wordexp (stapOptions, & words, WRDE_NOCMD|WRDE_UNDEF);
+  if (rc) 
+    { 
+      errWarn("cannot parse -s stap options");
+      return;
+    }
+  if (words.we_wordc+10 >= MAXSTAPARGC)  /* 10: padding for literal entries */
+    {
+      errWarn("too many -s options; MAXSTAPARGC");
+      return;
+    }
+
+  for (i=0; i<words.we_wordc; i++)
+    stapargv[stapargc++] = words.we_wordv[i];
+
+  stapargv[stapargc++] = "-k"; /* Need to keep temp files in order to package them up again. */
+
+  /* Process the saved command line arguments.  Avoid quoting/unquoting errors by
+     transcribing literally. */
+  stapargv[stapargc++] = "--client-options";
+  stapargv_freestart = stapargc;
+
+  for (i=1 ; ; i++)
+    {
+      char stapargfile[PATH_MAX];
+      FILE* argfile;
+      struct stat st;
+      char *arg;
+
+      if (stapargc >= MAXSTAPARGC) 
+        {
+          errWarn("too many stap options; MAXSTAPARGC");
+          return;
+        }
+
+      snprintf (stapargfile, PATH_MAX, "%s/argv%d", requestDirName, i);
+
+      rc = stat(stapargfile, & st);
+      if (rc) break;
+
+      arg = malloc (st.st_size+1);
+      if (!arg)
+        {
+          errWarn("stap arg malloc");
+          return;
+        }
+
+      argfile = fopen(stapargfile, "r");
+      if (! argfile)
+        {
+          errWarn("stap arg fopen");
+          return;
+        }
+
+      rc = fread(arg, 1, st.st_size, argfile);
+      if (rc != st.st_size)
+        {
+          errWarn("stap arg fread");
+          return;
+        }
+
+      arg[st.st_size] = '\0';
+      stapargv[stapargc++] = arg; /* freed later */
+      fclose (argfile);
+    }
+
+  snprintf (stapstdout, PATH_MAX, "%s/stdout", responseDirName);
+  snprintf (stapstderr, PATH_MAX, "%s/stderr", responseDirName);
+
+  stapargv[stapargc] = NULL; /* spawn_and_wait expects NULL termination */
+
+  /* Check for the unprivileged flag; we need this so that we can decide to sign the module. */
+  for (i=0; i<stapargc; i++)
+    if (strcmp (stapargv[i], "--unprivileged") == 0)
+      unprivileged=1;
+  /* NB: but it's not that easy!  What if an attacker passes
+     --unprivileged as some sort of argument-parameter, so that the
+     translator does not interpret it as an --unprivileged mode flag,
+     but something else?  Then it could generate unrestrained modules,
+     but silly we might still sign it, and let the attacker get away
+     with murder.  And yet we don't want to fully getopt-parse the
+     args here for duplication of effort.
+
+     So let's do a hack: forcefully add --unprivileged to stapargv[]
+     near the front in this case, something which a later option
+     cannot undo. */
+  if (unprivileged)
+    {
+      if (stapargc+1 >= MAXSTAPARGC) 
+        {
+          errWarn("too many stap options; MAXSTAPARGC");
+          return;
+        }
+
+      /* Shift all stapargv[] entries up one, including the NULL. */
+      for (i=stapargc; i>=1; i--)
+        stapargv[i+1]=stapargv[i];
+      stapargv_freestart ++; /* adjust for shift */
+
+      stapargv[1]="--unprivileged"; /* better not be resettable by later option */
+    }
+
+  /* All ready, let's run the translator! */
+  rc = spawn_and_wait (stapargv, "/dev/null", stapstdout, stapstderr, requestDirName);
+
+  /* Save the RC */
+  snprintf (staprc, PATH_MAX, "%s/rc", responseDirName);
+  f = fopen(staprc, "w");
+  if (f) 
+    {
+      /* best effort basis */
+      fprintf(f, "%d", rc);
+      fclose(f);
+    }
+
+  /* Parse output to extract the -k-saved temprary directory.
+     XXX: bletch. */
+  f = fopen(stapstderr, "r");
+  if (!f)
+    {
+      errWarn("stap stderr open");
+      return;
+    }
+
+  while (1)
+    {
+      char line[PATH_MAX];
+      char *l = fgets(line, PATH_MAX, f); /* NB: normally includes \n at end */
+      if (!l) break;
+      char key[]="Keeping temporary directory \"";
+
+      /* Look for line from main.cxx: s.keep_tmpdir */
+      if (strncmp(l, key, strlen(key)) == 0 &&
+          l[strlen(l)-2] == '"')  /* "\n */
+        { 
+          /* Move this directory under responseDirName.  We don't have to
+             preserve the exact stapXXXXXX suffix part, since stap-client
+             will accept anything ("stap......" regexp), and rewrite it
+             to a client-local string.
+             
+             We don't just symlink because then we'd have to
+             remember to delete it later anyhow. */
+          char *mvargv[10];
+          char *orig_staptmpdir = & l[strlen(key)];
+          char new_staptmpdir[PATH_MAX];
+
+          orig_staptmpdir[strlen(orig_staptmpdir)-2] = '\0'; /* Kill the closing "\n */
+          snprintf(new_staptmpdir, PATH_MAX, "%s/stap000000", responseDirName);
+          mvargv[0]="mv";
+          mvargv[1]=orig_staptmpdir;
+          mvargv[2]=new_staptmpdir;
+          mvargv[3]=NULL;
+          rc = spawn_and_wait (mvargv, NULL, NULL, NULL, NULL);
+          if (rc != PR_SUCCESS)
+            errWarn("stap tmpdir move");
+
+          /* In unprivileged mode, if we have a module built, we need to
+             sign the sucker. */
+          if (unprivileged) 
+            {
+              glob_t globber;
+              char pattern[PATH_MAX];
+              snprintf (pattern,PATH_MAX,"%s/*.ko", new_staptmpdir);
+              rc = glob (pattern, GLOB_ERR, NULL, &globber);
+              if (rc)
+                errWarn("stap tmpdir .ko glob");
+              else if (globber.gl_pathc != 1)
+                errWarn("stap tmpdir too many .ko globs");
+              else
+                {
+                  char *signargv [10];
+                  signargv[0] = STAP_PREFIX "/libexec/systemtap/stap-sign-module";
+                  signargv[1] = globber.gl_pathv[0];
+                  signargv[2] = dbdir;
+                  signargv[3] = NULL;
+                  rc = spawn_and_wait (signargv, NULL, NULL, NULL, NULL);
+                  if (rc != PR_SUCCESS)
+                    errWarn("stap-sign-module");
+                }
+            }
+        }
+
+      /* XXX: What about uprobes.ko? */
+    }
+
+  /* Free up all the arg string copies.  Note that the first few were alloc'd
+     by wordexp(), which wordfree() frees; others were hand-set to literal strings. */
+  for (i= stapargv_freestart; i<stapargc; i++)
+    free (stapargv[i]);
+  wordfree (& words);
+
+  /* Sorry about the inconvenience.  C string/file processing is such a pleasure. */
+}
+
+
+/* A frontend for posix_spawnp that waits for the child process and
+   returns overall success or failure. */
+static PRStatus spawn_and_wait (char ** argv,
+                                const char* fd0, const char* fd1, const char* fd2, const char *pwd)
+{ 
+  pid_t pid;
+  int rc;
+  int status;
+  extern char** environ;
+  posix_spawn_file_actions_t actions;
+  int dotfd = -1;
+
+#define CHECKRC(msg) do { if (rc) { errWarn(msg); return PR_FAILURE; } } while (0)
+
+  rc = posix_spawn_file_actions_init (& actions);
+  CHECKRC ("spawn file actions ctor");
+  if (fd0) {
+    rc = posix_spawn_file_actions_addopen(& actions, 0, fd0, O_RDONLY, 0600);
+    CHECKRC ("spawn file actions fd0");
+  }
+  if (fd1) {
+    rc = posix_spawn_file_actions_addopen(& actions, 1, fd1, O_WRONLY|O_CREAT, 0600);
+    CHECKRC ("spawn file actions fd1");
+  }
+  if (fd2) { 
+    rc = posix_spawn_file_actions_addopen(& actions, 2, fd2, O_WRONLY|O_CREAT, 0600);
+    CHECKRC ("spawn file actions fd2");
+  }
+
+  /* change temporarily to a directory if requested */
+  if (pwd)
+    {
+      dotfd = open (".", O_RDONLY);
+      if (dotfd < 0)
+        { 
+          errWarn ("spawn getcwd");
+          return PR_FAILURE;
+        }
+      
+      rc = chdir (pwd);
+      CHECKRC ("spawn chdir");
+    } 
+ 
+  rc = posix_spawnp (& pid, argv[0], & actions, NULL, argv, environ);
+  /* NB: don't react to rc!=0 right away; need to chdir back first. */
+
+  if (pwd && dotfd >= 0)
+    {
+      int subrc;
+      subrc = fchdir (dotfd);
+      subrc |= close (dotfd);
+      if (subrc) 
+        errWarn("spawn unchdir");
+    }
+
+  CHECKRC ("spawn");
+
+  rc = waitpid (pid, &status, 0);
+  if ((rc!=pid) || !WIFEXITED(status))
+    {
+      errWarn ("waitpid");
+      return PR_FAILURE;
+    }
+
+  rc = posix_spawn_file_actions_destroy (&actions);
+  CHECKRC ("spawn file actions dtor");
+
+  return WEXITSTATUS(status) ? PR_FAILURE : PR_SUCCESS;
+#undef CHECKRC
+}
+
+
+
 /* Function:  int handle_connection()
  *
- * Purpose:  Handle a connection to a socket.
- *
+ * Purpose: Handle a connection to a socket.  Copy in request zip
+ * file, process it, copy out response.  Temporary directories are
+ * created & destroyed here.
  */
 static SECStatus
 handle_connection(PRFileDesc *tcpSocket)
@@ -377,11 +674,16 @@ handle_connection(PRFileDesc *tcpSocket)
   SECStatus          secStatus = SECFailure;
   PRStatus           prStatus;
   PRSocketOptionData socketOption;
-  PRFileInfo         info;
-  char              *cmdline;
-  char              *stap_server_prefix;
   int                rc;
   char              *rc1;
+  char               tmpdir[PATH_MAX]; 
+  char               requestFileName[PATH_MAX];
+  char               requestDirName[PATH_MAX];
+  char               responseDirName[PATH_MAX];
+  char               responseFileName[PATH_MAX];
+  char              *argv[10]; /* we use fewer than these in all the posix_spawn's below. */
+
+  tmpdir[0]='\0'; /* prevent cleanup-time /bin/rm of uninitialized directory */
 
   /* Make sure the socket is blocking. */
   socketOption.option             = PR_SockOpt_Nonblocking;
@@ -410,50 +712,49 @@ handle_connection(PRFileDesc *tcpSocket)
       goto cleanup;
     }
 
-  /* Create a temporary files and directories.  */
-  memcpy (requestFileName + sizeof (requestFileName) - 1 - 6, "XXXXXX", 6);
-  rc = mkstemp(requestFileName);
-  if (rc == -1)
+  snprintf(tmpdir, PATH_MAX, "%s/stap-server.XXXXXX", getenv("TMPDIR") ?: "/tmp");
+  rc1 = mkdtemp(tmpdir);
+  if (! rc1)
     {
-      fprintf (stderr, "Could not create temporary file %s\n", requestFileName);
+      fprintf (stderr, "Could not create temporary directory %s\n", tmpdir);
       perror ("");
       secStatus = SECFailure;
+      tmpdir[0]=0; /* prevent /bin/rm */
       goto cleanup;
     }
 
-  memcpy (responseDirName + sizeof (responseDirName) - 1 - 6, "XXXXXX", 6);
-  rc1 = mkdtemp(responseDirName);
-  if (! rc1)
+  /* Create a temporary files names and directories.  */
+  snprintf (requestFileName, PATH_MAX, "%s/request.zip", tmpdir);
+
+  snprintf (requestDirName, PATH_MAX, "%s/request", tmpdir);
+  rc = mkdir(requestDirName, 0700);
+  if (rc)
     {
-      fprintf (stderr, "Could not create temporary directory %s\n", responseDirName);
+      fprintf (stderr, "Could not create temporary directory %s\n", requestDirName);
       perror ("");
       secStatus = SECFailure;
       goto cleanup;
     }
 
-  memcpy (responseZipName, responseDirName, sizeof (responseDirName) - 1);
-  memcpy (responseZipName + sizeof (responseZipName) - 1 - 6, "XXXXXX", 6);
-  rc = mkstemp(responseZipName);
-  if (rc == -1)
+  snprintf (responseDirName, PATH_MAX, "%s/response", tmpdir);
+  rc = mkdir(responseDirName, 0700);
+  if (rc)
     {
-      fprintf (stderr, "Could not create temporary file %s\n", responseZipName);
+      fprintf (stderr, "Could not create temporary directory %s\n", responseDirName);
       perror ("");
       secStatus = SECFailure;
-
-      /* Remove this so that the other temp files will get removed in cleanup.  */
-      prStatus = PR_RmDir (responseDirName);
-      if (prStatus != PR_SUCCESS)
-	errWarn ("PR_RmDir");
       goto cleanup;
     }
 
+  snprintf (responseFileName, PATH_MAX, "%s/response.zip", tmpdir);
+
   /* Read data from the socket.
    * If the user is requesting/requiring authentication, authenticate
    * the socket.  */
 #if DEBUG
   fprintf(stdout, "\nReading data from socket...\n\n");
 #endif
-  secStatus = readDataFromSocket(sslSocket);
+  secStatus = readDataFromSocket(sslSocket, requestFileName);
   if (secStatus != SECSuccess)
     goto cleanup;
 
@@ -467,32 +768,42 @@ handle_connection(PRFileDesc *tcpSocket)
     }
 #endif
 
-  /* Call the stap-server-request script.  */
-  stap_server_prefix = getenv("SYSTEMTAP_SERVER_SCRIPTS") ?: PKGLIBDIR;
-  cmdline = PORT_Alloc(strlen (stap_server_prefix) + sizeof ("/stap-server-request") + 1 +
-		       sizeof (requestFileName) + 1 +
-		       sizeof (responseDirName) + 1 +
-		       sizeof (responseZipName) + 1 +
-		       strlen (dbdir) + 1 +
-		       1 + strlen (stapOptions) + 1 +
-		       1);
-  if (! cmdline) {
-    errWarn ("PORT_Alloc");
-    secStatus = SECFailure;
-    goto cleanup;
-  }
-
-  sprintf (cmdline, "%s/stap-server-request %s %s %s %s '%s'", stap_server_prefix,
-	   requestFileName, responseDirName, responseZipName, dbdir,
-	   stapOptions);
-  rc = system (cmdline);
-
-  PR_Free (cmdline);
+  /* Unzip the request. */
+  argv[0]="unzip";
+  argv[1]="-d";
+  argv[2]=requestDirName;
+  argv[3]=requestFileName;
+  rc = spawn_and_wait(argv, NULL, NULL, NULL, NULL);
+  if (rc != PR_SUCCESS)
+    {
+      errWarn ("request unzip");
+      secStatus = SECFailure;
+      goto cleanup;
+    }
 
+  /* Handle the request zip file.  An error therein should still result
+     in a response zip file (containing stderr etc.) so we don't have to
+     have a result code here.  */
+  handleRequest(requestDirName, responseDirName);
+
+  /* Zip the response. */
+  argv[0]="zip";
+  argv[1]="-r";
+  argv[2]=responseFileName;
+  argv[3]=".";
+  argv[4]=NULL;
+  rc = spawn_and_wait(argv, NULL, NULL, NULL, responseDirName);
+  if (rc != PR_SUCCESS)
+    {
+      errWarn ("response zip");
+      secStatus = SECFailure;
+      goto cleanup;
+    }
+  
 #if DEBUG
   fprintf(stdout, "\nWriting data to socket...\n\n");
 #endif
-  secStatus = writeDataToSocket(sslSocket);
+  secStatus = writeDataToSocket(sslSocket, responseFileName);
 
 cleanup:
   /* Close down the socket. */
@@ -500,17 +811,16 @@ cleanup:
   if (prStatus != PR_SUCCESS)
     errWarn("PR_Close");
 
-  /* Attempt to remove temporary files, unless the temporary directory was
-     not deleted by the server script.  */
-  prStatus = PR_GetFileInfo(responseDirName, &info);
-  if (prStatus != PR_SUCCESS)
+  if (tmpdir[0]) 
     {
-      prStatus = PR_Delete (requestFileName);
-      if (prStatus != PR_SUCCESS)
-	errWarn ("PR_Delete");
-      prStatus = PR_Delete (responseZipName);
-      if (prStatus != PR_SUCCESS)
-	errWarn ("PR_Delete");
+      /* Remove the whole tmpdir and all that lies beneath. */
+      argv[0]="rm";
+      argv[1]="-r";
+      argv[2]=tmpdir;
+      argv[3]=NULL;
+      rc = spawn_and_wait(argv, NULL, NULL, NULL, NULL);
+      if (rc != PR_SUCCESS)
+        errWarn ("tmpdir cleanup");
     }
 
   return secStatus;
@@ -554,6 +864,9 @@ accept_connection(PRFileDesc *listenSocket)
 	      addr.inet.port);
       fflush (stdout);
 
+      /* XXX: alarm() or somesuch to set a timeout. */
+      /* XXX: fork() or somesuch to handle concurrent requests. */
+
       /* Accepted the connection, now handle it. */
       /*result =*/ handle_connection (tcpSocket);
 
@@ -564,16 +877,6 @@ accept_connection(PRFileDesc *listenSocket)
 	      (addr.inet.ip >> 24) & 0xff,
 	      addr.inet.port);
       fflush (stdout);
-
-#if 0 /* Not necessary */
-      if (result != SECSuccess)
-	{
-	  prStatus = PR_Close(tcpSocket);
-	  if (prStatus != PR_SUCCESS)
-	    exitErr("PR_Close");
-	  break;
-	}
-#endif
     }
 
 #if DEBUG
-- 
cgit 


From b703674d8fe87b0294f2df739e35545ab124a96e Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Fri, 15 Jan 2010 03:11:18 -0500
Subject: PR11105: Remove stap-server-request shell script.

* Makefile.am: Don't install it any more.
---
 Makefile.am         |   2 +-
 Makefile.in         |   2 +-
 stap-server-request | 512 ----------------------------------------------------
 3 files changed, 2 insertions(+), 514 deletions(-)
 delete mode 100755 stap-server-request

diff --git a/Makefile.am b/Makefile.am
index 3c2ddbe1..a7603fb2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -43,7 +43,7 @@ if BUILD_SERVER
 man_MANS += stap-client.8 stap-server.8 stap-authorize-server-cert.8
 pkglibexec_PROGRAMS += stap-client-connect stap-server-connect
 bin_SCRIPTS += stap-client stap-server stap-authorize-server-cert
-pkglibexec_SCRIPTS += stap-serverd stap-server-request stap-find-servers \
+pkglibexec_SCRIPTS += stap-serverd stap-find-servers \
 	       stap-start-server stap-find-or-start-server stap-stop-server
 endif
 endif
diff --git a/Makefile.in b/Makefile.in
index 016162ab..e655f8a2 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -47,7 +47,7 @@ pkglibexec_PROGRAMS = stapio$(EXEEXT) $(am__EXEEXT_2) $(am__EXEEXT_3)
 @BUILD_SERVER_TRUE@@BUILD_TRANSLATOR_TRUE@@HAVE_NSS_TRUE@am__append_8 = stap-client.8 stap-server.8 stap-authorize-server-cert.8
 @BUILD_SERVER_TRUE@@BUILD_TRANSLATOR_TRUE@@HAVE_NSS_TRUE@am__append_9 = stap-client-connect stap-server-connect
 @BUILD_SERVER_TRUE@@BUILD_TRANSLATOR_TRUE@@HAVE_NSS_TRUE@am__append_10 = stap-client stap-server stap-authorize-server-cert
-@BUILD_SERVER_TRUE@@BUILD_TRANSLATOR_TRUE@@HAVE_NSS_TRUE@am__append_11 = stap-serverd stap-server-request stap-find-servers \
+@BUILD_SERVER_TRUE@@BUILD_TRANSLATOR_TRUE@@HAVE_NSS_TRUE@am__append_11 = stap-serverd stap-find-servers \
 @BUILD_SERVER_TRUE@@BUILD_TRANSLATOR_TRUE@@HAVE_NSS_TRUE@	       stap-start-server stap-find-or-start-server stap-stop-server
 
 @BUILD_TRANSLATOR_FALSE@stap_DEPENDENCIES =
diff --git a/stap-server-request b/stap-server-request
deleted file mode 100755
index d1731895..00000000
--- a/stap-server-request
+++ /dev/null
@@ -1,512 +0,0 @@
-#!/bin/bash
-
-# Compile server for systemtap
-#
-# Copyright (C) 2008, 2009 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
-# Public License (GPL); either version 2, or (at your option) any
-# later version.
-
-# This script unpacks the tar file provided on stdin and uses the information
-# contained in the unpacked tree to build the requested systemtap kernel module.
-# This module is then written to stdout.
-
-# Catch ctrl-c and other termination signals
-trap 'terminate' SIGTERM SIGINT
-
-# Initialize the environment
-. ${PKGLIBEXECDIR}stap-env
-
-#-----------------------------------------------------------------------------
-# Helper functions.
-#-----------------------------------------------------------------------------
-# function: initialization
-function initialization {
-    # Initialization
-    stap_rc=0
-    wd=`pwd`
-
-    # Default options settings
-    p_phase=5
-    keep_temps=0
-    unprivileged=0
-    stap_options=
-
-    zip_client=$1
-    tmpdir_server=$2
-    zip_server=$3
-    ssl_db=$4
-    server_options="$5"
-
-    # This script is not intended to be called directly, so if the arguments
-    # aren't specified correctly, just exit with non-zero
-    test "X$tmpdir_server" != "X" || exit 2
-    test -d $tmpdir_server || exit 3
-    tmpdir_env=`dirname $tmpdir_server`
-
-    # Response file name.
-    test "X$zip_server" != "X" || exit 4
-    # Make sure the specified .zip file exists.
-    test -f $zip_server || exit 5
-
-    # Request file name.
-    test "X$zip_client" != "X" || exit 6
-    test -f $zip_client || exit 7
-
-    # Where is the ssl certificate/key database?
-    test "X$ssl_db" != "X" || exit 8
-    test -d $ssl_db || exit 9
-    nss_pw=$ssl_db/pw
-    test -f $nss_pw || exit 10
-
-    # What are the options that the server was started with?
-    eval parse_options "$server_options"
-
-    nss_cert=stap-server
-
-    touch $tmpdir_server/stdout
-    touch $tmpdir_server/stderr
-}
-
-# function: unpack_request
-#
-# Unpack the zip file received from the client and make the contents
-# available for use when running 'stap'
-function unpack_request {
-    cd $tmpdir_server
-
-    # Unpack the zip file.
-    unzip $zip_client > /dev/null || \
-	fatal "Cannot unpack zip archive $zip_client"
-
-    # Identify the client's request tree. The zip file should have expanded
-    # into a single directory named to match $stap_tmpdir_prefix_client.??????
-    # which should now be the only item in the current directory.
-    test "`ls | wc -l`" = 3 || \
-	fatal "Wrong number of files after expansion of client's zip file"
-
-    tmpdir_client=`ls | grep $stap_tmpdir_prefix_client.......\$`
-
-    test "X$tmpdir_client" != "X" || \
-	fatal "Client zip file did not expand as expected"
-
-    # Move the client's temp directory to a local temp location
-    local local_tmpdir_client=`mktemp -dt $stap_tmpdir_prefix_server.client.XXXXXX` || \
-	fatal "Cannot create temporary client request directory " $local_tmpdir_client
-    mv $tmpdir_client/* $local_tmpdir_client
-    rm -fr $tmpdir_client
-    tmpdir_client=$local_tmpdir_client
-}
-
-# function: check_request
-#
-# Examine the contents of the request to make sure that they are valid.
-function check_request {
-    # Work in the temporary directory provided by the client
-    cd $tmpdir_client
-
-    # Add the necessary info from files in our temporary directory.
-    cmdline=`read_data_file cmdline`
-    test "X$cmdline" != "X" || exit 1
-
-    eval parse_options "$cmdline"
-
-    client_sysinfo=`read_data_file sysinfo`
-    test "X$client_sysinfo" != "X" || exit 1
-
-    check_compatibility "$client_sysinfo" "`server_sysinfo`"
-}
-
-# function server_sysinfo
-#
-# Generate the server's sysinfo and echo it to stdout
-function server_sysinfo {
-    if test "X$sysinfo_server" = "X"; then
-	# Add some info from uname
-	sysinfo_server="`uname -r` `stap_get_arch`"
-    fi
-    echo "$sysinfo_server"
-}
-
-# function check_compaibility SYSINFO1 SYSINFO2
-#
-# Make sure that systemtap as described by SYSINFO1 and SYSINFO2 are compaible
-function check_compatibility {
-    # Compatibility is irrelevant. The client can choose any server
-    # it sees fit
-    return
-}
-
-# function: read_data_file PREFIX
-#
-# Find a file whose name is '$1' and whose first line
-# contents are '$1: .*'. Read and echo the data.
-function read_data_file {
-    test -f $1 || \
-	fatal "Data file $1 not found"
-
-    # Open the file
-    exec 3< $1
-
-    # Verify the first line of the file.
-    read <&3
-    line="$REPLY"
-    data=`expr "$line" : "$1: \\\(.*\\\)"`
-    if test "X$data" = "X"; then
-	fatal "Data in file $1 is incorrect"
-	return
-    fi
-
-    # Close the file
-    exec 3<&-
-
-    # Now read the entire file.
-    cat $1 | sed "s/$1: //"
-}
-
-# function: parse_options [ STAP-OPTIONS ]
-#
-# Examine the command line. We need not do much checking, but we do need to
-# parse all options in order to discover the ones we're interested in.
-function parse_options {
-    # We need to know in advance if --unprivileged was specified.
-    all_options=" ""$@"" "
-    token=`expr "$all_options" : '.* \(--unprivileged\) .*'`
-    if test "X$token" = "X--unprivileged"; then
-	unprivileged=1
-    fi
-
-    while test $# != 0
-    do
-	advance_p=0
-	dash_seen=0
-
-        # Start of a new token.
-	first_token=$1
-
-	# Process the option.
-	until test $advance_p != 0
-	do
-            # Identify the next option
-	    first_char=`expr "$first_token" : '\(.\).*'`
-	    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" : '.\(.\).*'`
-			if test "X$second_char" = "X-"; then
-			    advance_p=$(($advance_p + 1))
-			    stap_options="$stap_options $first_token"
-			    break
-			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" : '-\(.*\)'`
-			dash_seen=1
-			first_char=`expr "$first_token" : '\(.\).*'`
-		    fi
-		fi
-		if test $dash_seen = 0; then
-	            # The dash has not been seen. This is either the script file
-	            # name or an arument to be passed to the probe module.
-	            # If this is the first time, and -e has not been specified,
-	            # then it could be the name of the script file.
-		    if test "X$e_script" = "X" -a "X$script_file" = "X"; then
-			script_file="$first_token"
-		    fi
-		    if test "$first_char" != "'"; then
-			stap_options="$stap_options '$first_token'"
-		    else
-			stap_options="$stap_options $first_token"
-		    fi
-		    advance_p=$(($advance_p + 1))
-		    break
-		fi
-	    fi
-	    
-            # We are at the start of an option. Look at the first character.
-	    case $first_char in
-		a)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;
-		B)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;
-		d)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    ;;
-		D)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;
-		e)
-		    get_arg $first_token "$2"
-		    stap_options="$stap_options -$first_char '$stap_arg'"
-		    process_e "$stap_arg"
-		    ;;
-		I)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;	
-		k)
-		    keep_temps=1
-		    ;;
-		l)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    process_p 2
-		    ;;
-		L)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    process_p 2
-		    ;;
-		m)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;
-		o)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    ;;
-		p)
-		    get_arg $first_token $2
-		    process_p $stap_arg
-		    ;;
-		r)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;	
-		R)
-		    get_arg $first_token $2
-		    if test $unprivileged = 1; then
-			fatal "You can't specify -$first_char and --unprivileged together."
-		    else
-			stap_options="$stap_options -$first_char $stap_arg"
-		    fi
-		    ;;	
-		s)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    ;;	
-		S)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    ;;	
-		x)
-		    get_arg $first_token $2
-		    stap_options="$stap_options -$first_char $stap_arg"
-		    ;;
-		*)
-		    # An unknown flag. Ignore it.
-		    ;;
-	    esac
-
-	    if test $advance_p = 0; then
-	        # Just another flag character. Consume it.
-		stap_options="$stap_options -$first_char"
-		first_token=`expr "$first_token" : '.\(.*\)'`
-		if test "X$first_token" = "X"; then
-		    advance_p=$(($advance_p + 1))
-		fi
-	    fi
-	done
-
-        # Consume the arguments we just processed.
-	while test $advance_p != 0
-	do
-	    shift
-	    advance_p=$(($advance_p - 1))
-	done
-    done
-}
-
-# function: get_arg FIRSTWORD SECONDWORD
-#
-# Collect an argument to the given option
-function get_arg {
-    # Remove first character. Advance to the next token, if the first one
-    # is exhausted.
-    local first=`expr "$1" : '.\(.*\)'`
-    if test "X$first" = "X"; then
-	shift
-	advance_p=$(($advance_p + 1))
-	first=$1
-    fi
-    stap_arg="$first"
-    advance_p=$(($advance_p + 1))
-}
-
-# function: process_e ARGUMENT
-#
-# Process the -e flag.
-function process_e {
-    if test "X$e_script" = "X"; then
-	e_script="$1"
-	script_file=
-    fi
-}
-
-# function: process_p ARGUMENT
-#
-# Process the -p flag.
-function process_p {
-    if test $1 -ge 1 -a $1 -le 5; then
-	p_phase=$1
-    fi
-}
-
-# function: call_stap
-#
-# Call 'stap' with the options provided. Don't run past phase 4.
-function call_stap {
-    # Invoke systemtap.
-    # Use -k so we can return results to the client
-    # Limit to -p4. i.e. don't run the module
-    cd $tmpdir_client
-    if test $p_phase -gt 4; then
-	server_p_phase=4
-    else
-	server_p_phase=$p_phase
-    fi
-
-    eval ${stap_exec_prefix}stap "$stap_options" -k -p $server_p_phase \
-	>>  $tmpdir_server/stdout \
-	2>> $tmpdir_server/stderr
-
-    stap_rc=$?
-}
-
-# function: create_response
-#
-# Add information to the server's temp directory representing the response
-# to the client.
-function create_response {
-    cd $tmpdir_server
-
-    # Get the name of the stap temp directory, which was kept, from stderr.
-    tmpdir_line=`cat stderr | grep "Keeping temp"`
-    tmpdir_stap=`expr "$tmpdir_line" : '.*"\(.*\)".*'`
-
-    # Remove the message about keeping the stap temp directory from stderr, unless
-    # the user did request to keep it.
-    if test "X$tmpdir_stap" != "X"; then
-	if test $keep_temps != 1; then
-	    sed -i "/^Keeping temp/d" stderr
-	fi
-
-        # Add the contents of the stap temp directory to the server output directory
-	ln -s $tmpdir_stap `basename $tmpdir_stap`
-
-        # Sign the resulting module if --unprivileged was specified.
-	if test $unprivileged = 1 -a $p_phase -ge 4 -a $stap_rc = 0; then
-	    modname=$tmpdir_stap/`grep -m1 '^.*\.ko$' stdout`
-	    if test "X$modname" != "X"; then
-		${stap_pkglibexecdir}stap-sign-module $modname $ssl_db
-	    fi
-	fi
-    fi
-
-    # If the user specified -p5, remove the name of the kernel module from stdout.
-    if test $p_phase = 5; then
-	sed -i '/\.ko$/d' stdout
-    fi
-
-    # The return status of the stap command.
-    echo -n $stap_rc > rc
-}
-
-# function: package_response
-#
-# Package the server's temp directory into a form suitable for sending to the
-# client.
-function package_response {
-    cd $tmpdir_env
-
-    # Compress the server's temporary directory into a .zip archive.
-    (rm $zip_server && zip -r $zip_server `basename $tmpdir_server` > /dev/null)
-    return $?
-}
-
-# function: fatal [ MESSAGE ]
-#
-# Fatal error
-# Prints its arguments to stderr and exits
-function fatal {
-    echo "$0: ERROR:" "$@" >> $tmpdir_server/stderr
-    echo -n 1 > $tmpdir_server/rc
-    package_response
-    cleanup
-    exit 1
-}
-
-# Non fatal error
-# Prints its arguments to stderr but does not exit
-function error {
-    echo "$0: ERROR:" "$@" >> $tmpdir_server/stderr
-}
-
-# function cleanup
-#
-# Cleanup work files unless asked to keep them.
-function cleanup {
-    # Clean up.
-    cd $tmpdir_env
-    if test $keep_temps != 1; then
-	rm -fr $tmpdir_server
-	rm -fr $tmpdir_client
-	rm -fr $tmpdir_stap
-    fi
-}
-
-# function: terminate
-#
-# Terminate gracefully.
-function terminate {
-    # Clean up
-    cleanup
-    exit 1
-}
-
-#-----------------------------------------------------------------------------
-# Beginning of main line execution.
-#-----------------------------------------------------------------------------
-initialization "$@"
-unpack_request
-check_request
-call_stap
-create_response
-package_response
-cleanup
-
-exit 0
-- 
cgit 


From 36d1c134edc4bd4ee20225003041188c13b7f36f Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Fri, 15 Jan 2010 03:12:53 -0500
Subject: testsuite: fix wording of invalid-entry test group

---
 testsuite/systemtap.server/client_args.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testsuite/systemtap.server/client_args.exp b/testsuite/systemtap.server/client_args.exp
index 348f8cea..f41b91cb 100644
--- a/testsuite/systemtap.server/client_args.exp
+++ b/testsuite/systemtap.server/client_args.exp
@@ -1,11 +1,11 @@
-set test "Valid Server Client Arguments"
+set test "Invalid Server Client Arguments"
 
 # Test that stap on the server side will correctly accept/reject certain
 # arguments in unprivileged mode.
 set test_file $srcdir/systemtap.server/test.stp
 
 # Test invalid combinations.
-set error_regexp ".*You can't specify (-\[aBDImRr\], )*-\[aBDImRr\] when --unprivileged is specified.*"
+set error_regexp ".*You can't specify .* when --unprivileged is specified.*"
 
 set invalid_options [list \
   "--unprivileged --client-options -a i386" \
-- 
cgit 


From 86f99ad8206574dc6400d48563db58341cb50f52 Mon Sep 17 00:00:00 2001
From: "Frank Ch. Eigler" <fche@elastic.org>
Date: Fri, 15 Jan 2010 03:27:34 -0500
Subject: PR11105: remove extraneous \n from localized foo.stp script file name

---
 stap-client | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stap-client b/stap-client
index f8be3838..64452159 100755
--- a/stap-client
+++ b/stap-client
@@ -260,7 +260,7 @@ function parse_options {
 	else
 	    local_name="-"
 	fi
-	echo "script/$local_name" > "$tmpdir_client/argv$script_file_argc"
+	echo -n "script/$local_name" > "$tmpdir_client/argv$script_file_argc"
     fi
 
     # Processing based on final options settings
-- 
cgit