History

Fix access-after-free with statement expressions

The return value of statement expressions might refer to local symbols, so those can't be popped. The old error message always was just a band-aid, and since disabling it for pointer types it wasn't effective anyway. It also never considered that also the vtop->sym member might have referred to such symbols (see the testcase with the local static, that used to segfault). For fixing this (can be seen better with valgrind and SYM_DEBUG) simply leave local symbols of stmt exprs on the stack.

Michael Matz committed 10 years ago

Commit be6d8ff1 parent d0d25ec4 files · +76 −29Browse files

Unified · Split

4 files
  1. libtcc.c +2 −2
  2. tcc.h +1 −1
  3. tccgen.c +27 −26
  4. tests/tcctest.c +46 −0
libtcc.c+2−2
@@ -677,8 +677,8 @@ static int tcc_compile(TCCState *s1) · 676 unmodified lines
677677 s1->error_set_jmp_enabled = 0;
678678
679679 free_inline_functions(s1);
680− sym_pop(&global_stack, NULL);
681− sym_pop(&local_stack, NULL);
680+ sym_pop(&global_stack, NULL, 0);
681+ sym_pop(&local_stack, NULL, 0);
682682 return s1->nb_errors != 0 ? -1 : 0;
683683 }
684684
tcc.h+1−1
@@ -1099,7 +1099,7 @@ ST_INLN void sym_free(Sym *sym); · 1098 unmodified lines
10991099 ST_FUNC Sym *sym_push2(Sym **ps, int v, int t, long c);
11001100 ST_FUNC Sym *sym_find2(Sym *s, int v);
11011101 ST_FUNC Sym *sym_push(int v, CType *type, int r, int c);
1102−ST_FUNC void sym_pop(Sym **ptop, Sym *b);
1102+ST_FUNC void sym_pop(Sym **ptop, Sym *b, int keep);
11031103 ST_INLN Sym *struct_find(int v);
11041104 ST_INLN Sym *sym_find(int v);
11051105 ST_FUNC Sym *global_identifier_push(int v, int t, int c);
tccgen.c+27−26
@@ -358,17 +358,26 @@ static Sym *__sym_malloc(void) · 357 unmodified lines
358358 static inline Sym *sym_malloc(void)
359359 {
360360 Sym *sym;
361+#ifndef SYM_DEBUG
361362 sym = sym_free_first;
362363 if (!sym)
363364 sym = __sym_malloc();
364365 sym_free_first = sym->next;
365366 return sym;
367+#else
368+ sym = tcc_malloc(sizeof(Sym));
369+ return sym;
370+#endif
366371 }
367372
368373 ST_INLN void sym_free(Sym *sym)
369374 {
375+#ifndef SYM_DEBUG
370376 sym->next = sym_free_first;
371377 sym_free_first = sym;
378+#else
379+ tcc_free(sym);
380+#endif
372381 }
373382
374383 /* push, without hashing */
@@ -474,8 +483,9 @@ ST_FUNC Sym *global_identifier_push(int v, int t, int c) · 99 unmodified lines
474483 return s;
475484 }
476485
477−/* pop symbols until top reaches 'b' */
478−ST_FUNC void sym_pop(Sym **ptop, Sym *b)
486+/* pop symbols until top reaches 'b'. If KEEP is non-zero don't really
487+ pop them yet from the list, but do remove them from the token array. */
488+ST_FUNC void sym_pop(Sym **ptop, Sym *b, int keep)
479489 {
480490 Sym *s, *ss, **ps;
481491 TokenSym *ts;
@@ -495,10 +505,12 @@ ST_FUNC void sym_pop(Sym **ptop, Sym *b) · 13 unmodified lines
495505 ps = &ts->sym_identifier;
496506 *ps = s->prev_tok;
497507 }
498− sym_free(s);
508+ if (!keep)
509+ sym_free(s);
499510 s = ss;
500511 }
501− *ptop = b;
512+ if (!keep)
513+ *ptop = b;
502514 }
503515
504516 static void weaken_symbol(Sym *sym)
@@ -5406,28 +5418,17 @@ static void block(int *bsym, int *csym, int is_expr) · 4901 unmodified lines
54065418 }
54075419 /* pop locally defined labels */
54085420 label_pop(&local_label_stack, llabel);
5409− if(is_expr) {
5410− /* XXX: this solution makes only valgrind happy...
5411− triggered by gcc.c-torture/execute/20000917-1.c */
5412− Sym *p;
5413− switch(vtop->type.t & VT_BTYPE) {
5414− /* case VT_PTR: */
5415− /* this breaks a compilation of the linux kernel v2.4.26 */
5416− /* pmd_t *new = ({ __asm__ __volatile__("ud2\n") ; ((pmd_t *)1); }); */
5417− /* Look a commit a80acab: Display error on statement expressions with complex return type */
5418− /* A pointer is not a complex return type */
5419− case VT_STRUCT:
5420− case VT_ENUM:
5421− case VT_FUNC:
5422− for(p=vtop->type.ref;p;p=p->prev)
5423− if(p->prev==s)
5424− tcc_error("unsupported expression type");
5425− }
5426− }
54275421 /* pop locally defined symbols */
54285422 --local_scope;
5429− sym_pop(&local_stack, s);
5430−
5423+ /* In the is_expr case (a statement expression is finished here),
5424+ vtop might refer to symbols on the local_stack. Either via the
5425+ type or via vtop->sym. We can't pop those nor any that in turn
5426+ might be referred to. To make it easier we don't roll back
5427+ any symbols in that case; some upper level call to block() will
5428+ do that. We do have to remove such symbols from the lookup
5429+ tables, though. sym_pop will do that. */
5430+ sym_pop(&local_stack, s, is_expr);
5431+
54315432 /* Pop VLA frames and restore stack pointer if required */
54325433 if (vlas_in_scope > saved_vlas_in_scope) {
54335434 vla_sp_loc = saved_vlas_in_scope ? block_vla_sp_loc : vla_sp_root_loc;
@@ -5567,7 +5568,7 @@ static void block(int *bsym, int *csym, int is_expr) · 133 unmodified lines
55675568 gsym(a);
55685569 gsym_addr(b, c);
55695570 --local_scope;
5570− sym_pop(&local_stack, s);
5571+ sym_pop(&local_stack, s, 0);
55715572
55725573 } else
55735574 if (tok == TOK_DO) {
@@ -6592,7 +6593,7 @@ static void gen_function(Sym *sym) · 1018 unmodified lines
65926593 label_pop(&global_label_stack, NULL);
65936594 /* reset local stack */
65946595 local_scope = 0;
6595− sym_pop(&local_stack, NULL);
6596+ sym_pop(&local_stack, NULL, 0);
65966597 /* end of function */
65976598 /* patch symbol size */
65986599 ((ElfW(Sym) *)symtab_section->data)[sym->c].st_size =
tests/tcctest.c+46−0
@@ -2446,10 +2446,17 @@ void typeof_test(void) · 2445 unmodified lines
24462446 printf("a=%f b=%f c=%f\n", a, b, c);
24472447 }
24482448
2449+
2450+struct hlist_node;
2451+struct hlist_head {
2452+ struct hlist_node *first, *last;
2453+};
2454+
24492455 void statement_expr_test(void)
24502456 {
24512457 int a, i;
24522458
2459+ /* Basic stmt expr test */
24532460 a = 0;
24542461 for(i=0;i<10;i++) {
24552462 a += 1 +
@@ -2461,6 +2468,45 @@ void statement_expr_test(void) · 5 unmodified lines
24612468 }
24622469 printf("a=%d\n", a);
24632470
2471+ /* Test that symbols aren't freed prematurely.
2472+ With SYM_DEBUG valgrind will show a read from a freed
2473+ symbol, and tcc will show an (invalid) warning on the initialization
2474+ of 'ptr' below, if symbols are popped after the stmt expr. */
2475+ void *v = (void*)39;
2476+ typeof(({
2477+ (struct hlist_node *)v;
2478+ })) x;
2479+ typeof (x)
2480+ ptr = (struct hlist_node *)v;
2481+
2482+ /* This part used to segfault when symbols were popped prematurely.
2483+ The symbols for the static local would be overwritten with
2484+ helper symbols from the pre-processor expansions in between. */
2485+#define some_attr __attribute__((aligned(1)))
2486+#define tps(str) ({ \
2487+ static const char *t some_attr = str; \
2488+ t; \
2489+ })
2490+ printf ("stmtexpr: %s %s\n",
2491+ tps("somerandomlongstring"),
2492+ tps("anotherlongstring"));
2493+
2494+ /* Test that the three decls of 't' don't interact. */
2495+ int t = 40;
2496+ int b = ({ int t = 41; t; });
2497+ int c = ({ int t = 42; t; });
2498+
2499+ /* Test that aggregate return values work. */
2500+ struct hlist_head h
2501+ = ({
2502+ typedef struct hlist_head T;
2503+ long pre = 48;
2504+ T t = { (void*)43, (void*)44 };
2505+ long post = 49;
2506+ t;
2507+ });
2508+ printf ("stmtexpr: %d %d %d\n", t, b, c);
2509+ printf ("stmtexpr: %ld %ld\n", (long)h.first, (long)h.last);
24642510 }
24652511
24662512 void local_label_test(void)

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t