[Chilli] [PATCH] Fix: Memory leak of opt_run()

[Masashi Honma]

(2010/03/05 18:18), David Bird wrote:
> No, I haven't tried it in chilli, because it doesn't make much sense.
> The following will demonstrate my point.

I've tested your test code and gotten same result. But this code does
not describe what I want to say. I wrote some codes to describe my
thought. Try this patch and boot ChilliSpot with

# chilli -f -d

Index: main-opt.c
===================================================================
--- main-opt.c	(revision 297)
+++ main-opt.c	(working copy)
@@ -78,7 +78,17 @@
    char hostname[USERURLSIZE];
    int numargs;
    int ret = -1;
+  char *p;
  
+  //printf("argc=%d\n", argc);
+  printf("argv[5]=%p\n", argv[5]);
+  printf("argv[6]=%s\n", argv[6]);
+  p = (char *)strtol(argv[6], NULL, 16);
+  printf("p=%p\n", p);
+  fflush(stdout);
+  free(p);
+  printf("p was freed\n");
+  fflush(stdout);
    options_init();
  
    memset(&args_info, 0, sizeof(args_info));
Index: options.c
===================================================================
--- options.c	(revision 297)
+++ options.c	(working copy)
@@ -117,6 +117,7 @@
  static int opt_run(int argc, char **argv, int reload) {
    char **newargs;
    char file[128];
+  char messages[128];
    int status;
    int i;
  
@@ -133,10 +134,10 @@
      return status;
    }
  
-  if (!(newargs = calloc(1, sizeof(char *) * (argc + 4)))) {
+  if (!(newargs = calloc(1, sizeof(char *) * (argc + 16)))) {
      return -1;
    }
-
+
    for (i=1; i < argc; i++) {
      newargs[i] = argv[i];
    }
@@ -144,8 +145,15 @@
    newargs[0] = "chilli_opt";
    newargs[i++] = "-b";
    newargs[i++] = file;
-  newargs[i++] = reload ? "-r" : NULL;
+  //newargs[i++] = reload ? "-r" : NULL;
+  newargs[i++] = (char *)newargs;
+  printf("index=%d\n", i - 1);
+  printf("newargs[5]=%p\n", newargs[5]);
  
+  sprintf(messages, "%p", newargs[5]);
+  newargs[i++] = messages;
+  printf("newargs[6]=[%s]\n", newargs[6]);
+
    log_dbg("running chilli_opt on %s", file);
  
    if (execv(SBINDIR "/chilli_opt", newargs) != 0) {


I've gotten the result below on Linux

<stdout>
index=5
newargs[5]=0x9bf2028
newargs[6]=[0x9bf2028]
argv[5]=0xbf99beba
argv[6]=0x9bf2028
p=0x9bf2028

<dmesg>
[  409.621596] chilli_opt[3284]: segfault at 9bf2024 ip b763e071 sp bf99afb8 error 4 in libc-2.10.2.so[b75ce000+141000]

This means newargs pointer address is different between "before execv"
and "after execv". And maybe the free() of p resulted in segfault.
(I don't know why the address of stdout is 0x9bf2028 and dmesg is 9bf2024.)
This result can't say the p was already freed. I think you believe that p is freed
implicitly. Can you prove the p is freed ?