Re: [Tinycc-devel] [PATCH] invalid memory access in free_section()

[grischka]

David Raulo wrote:
> Hi,
>
> I am a user of the libtcc API; the code I'm working with comes from the
> tip of git://repo.or.cz/tinycc.git
> Here is a test case which causes a crash under MSVC; it seems to pass 
> under Linux, except if you take a closer look with valgrind
> (see attached file tcc_delete_test.c)
>
> What happens _I think_ is that in tcc_delete(), the recursive deletion 
> of the program sections can access the sh_flags member of a previously 
> deleted section.  With the test case, the call to free_section() on
> sections[6] (that is, .rel.text)  looks at s->link->sh_flag which
> belongs to the .symtab section; unfortunatly, the later was deleted
> already as sections[4].
>
> The proposed fix (attached) is to process the sections in 2 passes: 
> - one to recursively traverse the sections, and delete only the ones
> with the SHF_PRIVATE flag;
> - the second pass to delete the remaining sections.
>
> Is this fix correct?

I'm sure it is ;)

However since it starts looking really messy now (which was my fault
in the first place) I made a more linear approach to the issue:

http://repo.or.cz/w/tinycc.git?a=commitdiff;h=d247f924

Thanks for the report.

> Best regards,
> David.
>
> ------------------------------------------------------------------------
>
> --- tcc.c       2009/03/18 14:24:41     1.5
> +++ tcc.c       2009/03/20 10:46:47     1.6
> @@ -1301,14 +1301,21 @@
>      return sec;
>  }
>  
> -static void free_section(Section *s)
> +static void free_private_section(Section *s)
>  {
> -    if (s->link && (s->link->sh_flags & SHF_PRIVATE))
> -        free_section(s->link);
> -    if (s->hash && (s->hash->sh_flags & SHF_PRIVATE))
> -        s->hash->link = NULL, free_section(s->hash);
> -    tcc_free(s->data);
> -    tcc_free(s);
> +    if (s->link && (s->link->sh_flags & SHF_PRIVATE)) {
> +        free_private_section(s->link);
> +        s->link = 0;
> +    }
> +    if (s->hash && (s->hash->sh_flags & SHF_PRIVATE)) {
> +        s->hash->link = NULL;
> +        free_private_section(s->hash);
> +        s->hash = 0;
> +    }
> +    if (s->sh_flags & SHF_PRIVATE) {
> +        tcc_free(s->data);
> +        tcc_free(s);
> +    }
>  }
>  
>  /* realloc section and set its content to zero */
> @@ -10437,10 +10444,15 @@
>      tcc_cleanup();
>  
>      /* free all sections */
> -    free_section(s1->dynsymtab_section);
> +    free_private_section(s1->dynsymtab_section);
>  
>      for(i = 1; i < s1->nb_sections; i++)
> -        free_section(s1->sections[i]);
> +        free_private_section(s1->sections[i]);
> +    for(i = 1; i < s1->nb_sections; i++) {
> +        tcc_free(s1->sections[i]->data);
> +        tcc_free(s1->sections[i]);
> +    }
> +
>      tcc_free(s1->sections);
>      
>      /* free any loaded DLLs */
>
>
> ------------------------------------------------------------------------
>
> #include <stdio.h>
> #include <libtcc.h>
>
> void run(char* code, char* name)
> {
>   TCCState* state = tcc_new();
>   unsigned long addr = 0;
>   int res = 0;
>   double(*f)(double) = 0;
>   res = tcc_set_output_type(state, TCC_OUTPUT_MEMORY);
>   res = tcc_compile_string(state, code);
>   res = tcc_relocate(state);
>   res = tcc_get_symbol(state, &addr, name);
>   f = addr;
>   printf("f(0)=%g\n", f(0));
>   printf("f(1)=%g\n", f(1));
>   tcc_delete(state);
> }
>
> int main()
> {
>   char* code = "double func(double x) { return (1.0-x); }";
>   run(code, "func");
>   return 0;
> }
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Tinycc-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/tinycc-devel