Browse Source

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>
Pierre Marsais 3 months ago
parent
commit
7df9a0049e
4 changed files with 31 additions and 29 deletions
  1. 1
    3
      defs.h
  2. 22
    11
      stage_output.c
  3. 7
    14
      strace.c
  4. 1
    1
      syscall.c

+ 1
- 3
defs.h View File

@@ -204,9 +204,7 @@ struct tcb {
204 204
 	int sys_func_rval;	/* Syscall entry parser's return value */
205 205
 	int curcol;		/* Output column for this process */
206 206
 	FILE *outf;		/* Output file for this process */
207
-	FILE *real_outf;	/* Backup for real outf while staging */
208
-	char *memfptr;
209
-	size_t memfloc;
207
+	struct staged_output_data *staged_output_data;
210 208
 
211 209
 	const char *auxstr;	/* Auxiliary info from syscall (see RVAL_STR) */
212 210
 	void *_priv_data;	/* Private data for syscall decoding functions */

+ 22
- 11
stage_output.c View File

@@ -14,14 +14,21 @@
14 14
 
15 15
 #include "defs.h"
16 16
 
17
+struct staged_output_data {
18
+	char *memfptr;
19
+	size_t memfloc;
20
+	FILE *real_outf;	/* Backup for real outf while staging */
21
+};
22
+
17 23
 FILE *
18 24
 strace_open_memstream(struct tcb *tcp)
19 25
 {
20 26
 	FILE *fp = NULL;
21 27
 
22 28
 #if HAVE_OPEN_MEMSTREAM
23
-	tcp->memfptr = NULL;
24
-	fp = open_memstream(&tcp->memfptr, &tcp->memfloc);
29
+	tcp->staged_output_data = xmalloc(sizeof(*tcp->staged_output_data));
30
+	fp = open_memstream(&tcp->staged_output_data->memfptr,
31
+			    &tcp->staged_output_data->memfloc);
25 32
 	if (!fp)
26 33
 		perror_msg_and_die("open_memstream");
27 34
 	/*
@@ -31,7 +38,7 @@ strace_open_memstream(struct tcb *tcp)
31 38
 	fflush(fp);
32 39
 
33 40
 	/* Store the FILE pointer for later restauration. */
34
-	tcp->real_outf = tcp->outf;
41
+	tcp->staged_output_data->real_outf = tcp->outf;
35 42
 	tcp->outf = fp;
36 43
 #endif
37 44
 
@@ -42,7 +49,7 @@ void
42 49
 strace_close_memstream(struct tcb *tcp, bool publish)
43 50
 {
44 51
 #if HAVE_OPEN_MEMSTREAM
45
-	if (!tcp->real_outf) {
52
+	if (!tcp->staged_output_data) {
46 53
 		debug_msg("memstream already closed");
47 54
 		return;
48 55
 	}
@@ -50,15 +57,19 @@ strace_close_memstream(struct tcb *tcp, bool publish)
50 57
 	if (fclose(tcp->outf))
51 58
 		perror_msg("fclose(tcp->outf)");
52 59
 
53
-	tcp->outf = tcp->real_outf;
54
-	tcp->real_outf = NULL;
55
-	if (tcp->memfptr) {
60
+	tcp->outf = tcp->staged_output_data->real_outf;
61
+	if (tcp->staged_output_data->memfptr) {
56 62
 		if (publish)
57
-			fputs_unlocked(tcp->memfptr, tcp->outf);
63
+			fputs_unlocked(tcp->staged_output_data->memfptr,
64
+				       tcp->outf);
58 65
 		else
59
-			debug_msg("syscall output dropped: %s", tcp->memfptr);
60
-		free(tcp->memfptr);
61
-		tcp->memfptr = NULL;
66
+			debug_msg("syscall output dropped: %s",
67
+				  tcp->staged_output_data->memfptr);
68
+
69
+		free(tcp->staged_output_data->memfptr);
70
+		tcp->staged_output_data->memfptr = NULL;
62 71
 	}
72
+	free(tcp->staged_output_data);
73
+	tcp->staged_output_data = NULL;
63 74
 #endif
64 75
 }

+ 7
- 14
strace.c View File

@@ -604,7 +604,7 @@ printleader(struct tcb *tcp)
604 604
 
605 605
 	if (printing_tcp) {
606 606
 		set_current_tcp(printing_tcp);
607
-		if (tcp->real_outf == NULL && printing_tcp->curcol != 0 &&
607
+		if (!tcp->staged_output_data && printing_tcp->curcol != 0 &&
608 608
 		    (followfork < 2 || printing_tcp == tcp)) {
609 609
 			/*
610 610
 			 * 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)
2094 2094
 	FILE *fp = execve_thread->outf;
2095 2095
 	execve_thread->outf = tcp->outf;
2096 2096
 	tcp->outf = fp;
2097
-	if (execve_thread->real_outf || tcp->real_outf) {
2098
-		char *memfptr;
2099
-		size_t memfloc;
2100
-
2101
-		fp = execve_thread->real_outf;
2102
-		execve_thread->real_outf = tcp->real_outf;
2103
-		tcp->real_outf = fp;
2104
-		memfptr = execve_thread->memfptr;
2105
-		execve_thread->memfptr = tcp->memfptr;
2106
-		tcp->memfptr = memfptr;
2107
-		memfloc = execve_thread->memfloc;
2108
-		execve_thread->memfloc = tcp->memfloc;
2109
-		tcp->memfloc = memfloc;
2097
+	if (execve_thread->staged_output_data || tcp->staged_output_data) {
2098
+		struct staged_output_data *staged_output_data;
2099
+
2100
+		staged_output_data = execve_thread->staged_output_data;
2101
+		execve_thread->staged_output_data = tcp->staged_output_data;
2102
+		tcp->staged_output_data = staged_output_data;
2110 2103
 	}
2111 2104
 
2112 2105
 	/* And their column positions */

+ 1
- 1
syscall.c View File

@@ -715,7 +715,7 @@ print_syscall_resume(struct tcb *tcp)
715 715
 	 * "strace -ff -oLOG test/threaded_execve" corner case.
716 716
 	 * It's the only case when -ff mode needs reprinting.
717 717
 	 */
718
-	if ((followfork < 2 && printing_tcp != tcp && tcp->real_outf == NULL)
718
+	if ((followfork < 2 && printing_tcp != tcp && !tcp->staged_output_data)
719 719
 	    || (tcp->flags & TCB_REPRINT)) {
720 720
 		tcp->flags &= ~TCB_REPRINT;
721 721
 		printleader(tcp);

Loading…
Cancel
Save