On Friday 31 May 2013 20:34:16 Dag Wieers wrote:
> --- busybox-1.21.0/procps/pstree.c.orig 2013-06-01 02:27:14.008530872 +0200
> +++ busybox-1.21.0/procps/pstree.c      2013-06-01 02:31:36.483537110 +0200
> @@ -342,7 +342,7 @@
>   static void handle_thread(const char *comm, pid_t pid, pid_t ppid, uid_t
> uid) {
>          char threadname[COMM_LEN + 2];
> -       sprintf(threadname, "{%.*s}", COMM_LEN - 2, comm);
> +       sprintf(threadname, "{%.*s}", COMM_LEN - 1, comm);
>          add_proc(threadname, pid, ppid, uid/*, 1*/);
>   }

hmm, your change is correct all by itself (but really it should be using 
sizeof()), but i don't think the current code is correct.  it *should* be 
truncating 2 bytes from the comm name.

we have this global struct:
typedef struct proc {
    char comm[COMM_LEN + 1];
...
} PROC;

and add_proc does:
static void add_proc(const char *comm, pid_t pid, pid_t ppid,
            uid_t uid /*, char isthread*/)
{   
    PROC *this, *parent;

    this = find_proc(pid);
    if (!this)
        this = new_proc(comm, pid, uid);
    else {
        strcpy(this->comm, comm);
...

and new_proc does:
static PROC *new_proc(const char *comm, pid_t pid, uid_t uid)
{
    PROC *new = xzalloc(sizeof(*new));

    strcpy(new->comm, comm);
...

so PROC provides storage for COMM_LEN printable bytes (the +1 is for the 
trailing NUL).  handle_thread sets up storage for COMM_LEN+1 printable bytes.  
then it does a strcpy which means it clobbers 1 too many bytes in the PROC 
struct.

try this patch instead:
diff --git a/procps/pstree.c b/procps/pstree.c
index 8ba3079..c30f384 100644
--- a/procps/pstree.c
+++ b/procps/pstree.c
@@ -34,8 +34,15 @@
 
 struct child;
 
+#ifdef ENABLE_FEATURE_SHOW_THREADS
+/* For threads, we add {...} around the comm, so we need two extra bytes */
+# define COMM_DISP_LEN (COMM_LEN + 2)
+#else
+# define COMM_DISP_LEN COMM_LEN
+#endif
+
 typedef struct proc {
-       char comm[COMM_LEN + 1];
+       char comm[COMM_DISP_LEN + 1];
 //     char flags; - unused, delete?
        pid_t pid;
        uid_t uid;
@@ -341,8 +348,8 @@ static void dump_by_user(PROC *current, uid_t uid)
 #if ENABLE_FEATURE_SHOW_THREADS
 static void handle_thread(const char *comm, pid_t pid, pid_t ppid, uid_t uid)
 {
-       char threadname[COMM_LEN + 2];
-       sprintf(threadname, "{%.*s}", COMM_LEN - 2, comm);
+       char threadname[COMM_DISP_LEN + 1];
+       sprintf(threadname, "{%.*s}", sizeof(threadname) - 1, comm);
        add_proc(threadname, pid, ppid, uid/*, 1*/);
 }
 #endif
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to