Fix invalid free in trace_close_memstream

In maybe_switch_tcbs we exchange the pointers to the memstream's buffers
between 2 tcb, however the libc doesn't know and keeps updating the
tcb->memfptr as if the exchange didn't happen.  This leads to
unsynchronized tcb->memfptr and tcb->outf and invalid frees.
Adding a new indirection fixes the problem.

* stage_output.c (struct staged_output_data): New struct.
(strace_open_memstream, strace_close_memstream): Use it.
* defs.h (struct tcb): Replace real_outf, memfptr, and memfloc
with a pointer to struct staged_output_data.
* strace.c (maybe_switch_tcbs): Use it.
* syscall.c (print_syscall_resume): Ditto.

Signed-off-by: Pierre Marsais <pierre.marsais@lse.epita.fr>

diff --git a/defs.h b/defs.h
index 51622c05d63eea23b32baaa0cf8fc68dbc879660..a9ef361a11837004046de9afadbcfa90938f7345 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -204,9 +204,7 @@ struct tcb {
        int sys_func_rval;      /* Syscall entry parser's return value */
        int curcol;             /* Output column for this process */
        FILE *outf;             /* Output file for this process */
-       FILE *real_outf;        /* Backup for real outf while staging */
-       char *memfptr;
-       size_t memfloc;
+       struct staged_output_data *staged_output_data;
 
        const char *auxstr;     /* Auxiliary info from syscall (see RVAL_STR) */
        void *_priv_data;       /* Private data for syscall decoding functions */

diff --git a/stage_output.c b/stage_output.c
index 971714b7b407ccd245b6895288fb32960e7798a6..d9d0352bb4094a3454f537f3775814e790faaa35 100644 (file)
--- a/stage_output.c
+++ b/stage_output.c
@@ -14,14 +14,21 @@
 
 #include "defs.h"
 
+struct staged_output_data {
+       char *memfptr;
+       size_t memfloc;
+       FILE *real_outf;        /* Backup for real outf while staging */
+};
+
 FILE *
 strace_open_memstream(struct tcb *tcp)
 {
        FILE *fp = NULL;
 
 #if HAVE_OPEN_MEMSTREAM
-       tcp->memfptr = NULL;
-       fp = open_memstream(&tcp->memfptr, &tcp->memfloc);
+       tcp->staged_output_data = xmalloc(sizeof(*tcp->staged_output_data));
+       fp = open_memstream(&tcp->staged_output_data->memfptr,
+                           &tcp->staged_output_data->memfloc);
        if (!fp)
                perror_msg_and_die("open_memstream");
        /*
@@ -31,7 +38,7 @@ strace_open_memstream(struct tcb *tcp)
        fflush(fp);
 
        /* Store the FILE pointer for later restauration. */
-       tcp->real_outf = tcp->outf;
+       tcp->staged_output_data->real_outf = tcp->outf;
        tcp->outf = fp;
 #endif
 
@@ -42,7 +49,7 @@ void
 strace_close_memstream(struct tcb *tcp, bool publish)
 {
 #if HAVE_OPEN_MEMSTREAM
-       if (!tcp->real_outf) {
+       if (!tcp->staged_output_data) {
                debug_msg("memstream already closed");
                return;
        }
@@ -50,15 +57,19 @@ strace_close_memstream(struct tcb *tcp, bool publish)
        if (fclose(tcp->outf))
                perror_msg("fclose(tcp->outf)");
 
-       tcp->outf = tcp->real_outf;
-       tcp->real_outf = NULL;
-       if (tcp->memfptr) {
+       tcp->outf = tcp->staged_output_data->real_outf;
+       if (tcp->staged_output_data->memfptr) {
                if (publish)
-                       fputs_unlocked(tcp->memfptr, tcp->outf);
+                       fputs_unlocked(tcp->staged_output_data->memfptr,
+                                      tcp->outf);
                else
-                       debug_msg("syscall output dropped: %s", tcp->memfptr);
-               free(tcp->memfptr);
-               tcp->memfptr = NULL;
+                       debug_msg("syscall output dropped: %s",
+                                 tcp->staged_output_data->memfptr);
+
+               free(tcp->staged_output_data->memfptr);
+               tcp->staged_output_data->memfptr = NULL;
        }
+       free(tcp->staged_output_data);
+       tcp->staged_output_data = NULL;
 #endif
 }

diff --git a/strace.c b/strace.c
index afa841a7617b62acec302260706fab03812e63e7..5e8b0417bc66225ff36df2aa7ea56161b3a0d525 100644 (file)
--- a/strace.c
+++ b/strace.c
@@ -604,7 +604,7 @@ printleader(struct tcb *tcp)
 
        if (printing_tcp) {
                set_current_tcp(printing_tcp);
-               if (tcp->real_outf == NULL && printing_tcp->curcol != 0 &&
+               if (!tcp->staged_output_data && printing_tcp->curcol != 0 &&
                    (followfork < 2 || printing_tcp == tcp)) {
                        /*
                         * case 1: we have a shared log (i.e. not -ff), and last line
@@ -2094,19 +2094,12 @@ maybe_switch_tcbs(struct tcb *tcp, const int pid)
        FILE *fp = execve_thread->outf;
        execve_thread->outf = tcp->outf;
        tcp->outf = fp;
-       if (execve_thread->real_outf || tcp->real_outf) {
-               char *memfptr;
-               size_t memfloc;
-
-               fp = execve_thread->real_outf;
-               execve_thread->real_outf = tcp->real_outf;
-               tcp->real_outf = fp;
-               memfptr = execve_thread->memfptr;
-               execve_thread->memfptr = tcp->memfptr;
-               tcp->memfptr = memfptr;
-               memfloc = execve_thread->memfloc;
-               execve_thread->memfloc = tcp->memfloc;
-               tcp->memfloc = memfloc;
+       if (execve_thread->staged_output_data || tcp->staged_output_data) {
+               struct staged_output_data *staged_output_data;
+
+               staged_output_data = execve_thread->staged_output_data;
+               execve_thread->staged_output_data = tcp->staged_output_data;
+               tcp->staged_output_data = staged_output_data;
        }
 
        /* And their column positions */

diff --git a/syscall.c b/syscall.c
index 8203ef06c87bb0a62eeaff8ab9c33608335b8693..20d04d0d6ff52bd95ed88762c5be4d273ae26670 100644 (file)
--- a/syscall.c
+++ b/syscall.c
@@ -715,7 +715,7 @@ print_syscall_resume(struct tcb *tcp)
         * "strace -ff -oLOG test/threaded_execve" corner case.
         * It's the only case when -ff mode needs reprinting.
         */
-       if ((followfork < 2 && printing_tcp != tcp && tcp->real_outf == NULL)
+       if ((followfork < 2 && printing_tcp != tcp && !tcp->staged_output_data)
            || (tcp->flags & TCB_REPRINT)) {
                tcp->flags &= ~TCB_REPRINT;
                printleader(tcp);