Patchwork Overwrite argv to set process title, eliminating 16-character prctl() limit.

login
register
mail settings
Submitter John Morrissey
Date Nov. 7, 2010, 3:44 p.m.
Message ID <1289144652-23463-1-git-send-email-jwm@horde.net>
Download mbox | patch
Permalink /patch/70352/
State New
Headers show

Comments

John Morrissey - Nov. 7, 2010, 3:44 p.m.
Linux seems to maintain the length of the original args, even when the new
args are shorter and NULL-terminated, so the trailing whitespace in ps(1)
output is probably unavoidable. I've seen the same result with other daemons
that overwrite argv.

Signed-off-by: John Morrissey <jwm@horde.net>
---
 os-posix.c      |   32 ++++++++++++++++----------------
 qemu-os-posix.h |    2 +-
 qemu-os-win32.h |    3 ++-
 vl.c            |   11 +++++++++--
 4 files changed, 28 insertions(+), 20 deletions(-)
Andreas Färber - Nov. 7, 2010, 6:27 p.m.
Am 07.11.2010 um 16:44 schrieb John Morrissey:

> Linux seems to maintain the length of the original args, even when  
> the new
> args are shorter and NULL-terminated, so the trailing whitespace in  
> ps(1)
> output is probably unavoidable. I've seen the same result with other  
> daemons
> that overwrite argv.
>
> Signed-off-by: John Morrissey <jwm@horde.net>
> ---

> diff --git a/os-posix.c b/os-posix.c
> index 38c29d1..3ddf7e8 100644
> --- a/os-posix.c
> +++ b/os-posix.c

> @@ -149,20 +145,24 @@ char *os_find_datadir(const char *argv0)
> #undef SHARE_SUFFIX
> #undef BUILD_SUFFIX
>
> -void os_set_proc_name(const char *s)
> +void os_set_proc_name(int argc, char **argv, const char *name)
> {
> -#if defined(PR_SET_NAME)
> -    char name[16];
> -    if (!s)
> +#ifdef CONFIG_LINUX

Is PR_SET_NAME defined outside Linux? If so, then your patch removes  
the limited functionality for those platforms completely.

Andreas
John Morrissey - Nov. 7, 2010, 7:42 p.m.
On Sun, Nov 07, 2010 at 07:27:22PM +0100, Andreas Färber wrote:
> Am 07.11.2010 um 16:44 schrieb John Morrissey:
> >@@ -149,20 +145,24 @@ char *os_find_datadir(const char *argv0)
> >#undef SHARE_SUFFIX
> >#undef BUILD_SUFFIX
> >
> >-void os_set_proc_name(const char *s)
> >+void os_set_proc_name(int argc, char **argv, const char *name)
> >{
> >-#if defined(PR_SET_NAME)
> >-    char name[16];
> >-    if (!s)
> >+#ifdef CONFIG_LINUX
> 
> Is PR_SET_NAME defined outside Linux? If so, then your patch removes
> the limited functionality for those platforms completely.

PR_SET_NAME is defined in sys/prctl.h which, before this patch, was
conditionally included only on Linux platforms (CONFIG_LINUX).

Additionally, prctl(2) says:

  CONFORMING TO
         This  call  is  Linux-specific.

john
Torsten Förtsch - Nov. 7, 2010, 8:49 p.m.
Hi,

On Sunday, November 07, 2010 16:44:12 John Morrissey wrote:
> -    if (prctl(PR_SET_NAME, name)) {
> -        perror("unable to change process name");
> -        exit(1);
> -    }
> +
> +    last_argv_byte = argv[argc - 1] + strlen(argv[argc - 1]);
> +
> +    len = snprintf(argv[0], last_argv_byte - argv[0], "%s", name);
> +
> +    p = &argv[0][len];
> +    while (p <= last_argv_byte)
> +        *p++ = '\0';
> +    for (i = 1; i < argc; ++i)
> +        argv[i] = (char *) "";

I am quite new to the list but why not do both call prctl(PR_SET_NAME, name) 
and overwrite argv?

The point is some tools read /proc/PID/cmdline but others the name field in 
/proc/PID/status. The former is changed by overwriting argv the latter by 
prctl.

Torsten Förtsch

Patch

diff --git a/os-posix.c b/os-posix.c
index 38c29d1..3ddf7e8 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -39,10 +39,6 @@ 
 #include "net/slirp.h"
 #include "qemu-options.h"
 
-#ifdef CONFIG_LINUX
-#include <sys/prctl.h>
-#endif
-
 #ifdef CONFIG_EVENTFD
 #include <sys/eventfd.h>
 #endif
@@ -149,20 +145,24 @@  char *os_find_datadir(const char *argv0)
 #undef SHARE_SUFFIX
 #undef BUILD_SUFFIX
 
-void os_set_proc_name(const char *s)
+void os_set_proc_name(int argc, char **argv, const char *name)
 {
-#if defined(PR_SET_NAME)
-    char name[16];
-    if (!s)
+#ifdef CONFIG_LINUX
+    char *last_argv_byte, *p;
+    int len, i;
+
+    if (!name)
         return;
-    name[sizeof(name) - 1] = 0;
-    strncpy(name, s, sizeof(name));
-    /* Could rewrite argv[0] too, but that's a bit more complicated.
-       This simple way is enough for `top'. */
-    if (prctl(PR_SET_NAME, name)) {
-        perror("unable to change process name");
-        exit(1);
-    }
+
+    last_argv_byte = argv[argc - 1] + strlen(argv[argc - 1]);
+
+    len = snprintf(argv[0], last_argv_byte - argv[0], "%s", name);
+
+    p = &argv[0][len];
+    while (p <= last_argv_byte)
+        *p++ = '\0';
+    for (i = 1; i < argc; ++i)
+        argv[i] = (char *) "";
 #else
     fprintf(stderr, "Change of process name not supported by your OS\n");
     exit(1);
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 353f878..b0cf993 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -31,7 +31,7 @@  static inline void os_host_main_loop_wait(int *timeout)
 }
 
 void os_set_line_buffering(void);
-void os_set_proc_name(const char *s);
+void os_set_proc_name(int argc, char **argv, const char *name);
 void os_setup_signal_handling(void);
 void os_daemonize(void);
 void os_setup_post(void);
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 1a07e5e..c618362 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -46,7 +46,8 @@  static inline void os_setup_signal_handling(void) {}
 static inline void os_daemonize(void) {}
 static inline void os_setup_post(void) {}
 void os_set_line_buffering(void);
-static inline void os_set_proc_name(const char *dummy) {}
+static inline void os_set_proc_name(int argc, char **argv,
+                                    const char *dummy) {}
 
 #if !defined(EPROTONOSUPPORT)
 # define EPROTONOSUPPORT EINVAL
diff --git a/vl.c b/vl.c
index c58583d..4b203cf 100644
--- a/vl.c
+++ b/vl.c
@@ -1772,7 +1772,11 @@  static const QEMUOption *lookup_opt(int argc, char **argv,
         optarg = NULL;
     }
 
-    *poptarg = optarg;
+    if (optarg != NULL) {
+        *poptarg = qemu_strdup(optarg);
+    } else {
+        *poptarg = NULL;
+    }
     *poptind = optind;
 
     return popt;
@@ -1800,6 +1804,7 @@  int main(int argc, char **argv, char **envp)
     int tb_size;
     const char *pid_file = NULL;
     const char *incoming = NULL;
+    const char *process_name = NULL;
     int show_vnc_port = 0;
     int defconfig = 1;
 
@@ -2493,7 +2498,7 @@  int main(int argc, char **argv, char **envp)
 			    exit(1);
 			}
 			p += 8;
-			os_set_proc_name(p);
+			process_name = p;
 		     }	
 		 }	
                 break;
@@ -2719,6 +2724,8 @@  int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    os_set_proc_name(argc, argv, process_name);
+
     if (kvm_allowed) {
         int ret = kvm_init(smp_cpus);
         if (ret < 0) {