don't leak memory if exec() arguments are invalid.

2 files changed, 27 insertions, 4 deletions

diff --git a/kernel/sysfile.c b/kernel/sysfile.c
index 5b09d93..7768d20 100644
--- a/kernel/sysfile.c
+++ b/kernel/sysfile.c
@@ -421,10 +421,10 @@ sys_exec(void)
   memset(argv, 0, sizeof(argv));
   for(i=0;; i++){
     if(i >= NELEM(argv)){
-      return -1;
+      goto bad;
     }
     if(fetchaddr(uargv+sizeof(uint64)*i, (uint64*)&uarg) < 0){
-      return -1;
+      goto bad;
     }
     if(uarg == 0){
       argv[i] = 0;
@@ -434,7 +434,7 @@ sys_exec(void)
     if(argv[i] == 0)
       panic("sys_exec kalloc");
     if(fetchstr(uarg, argv[i], PGSIZE) < 0){
-      return -1;
+      goto bad;
     }
   }
 
@@ -444,6 +444,11 @@ sys_exec(void)
     kfree(argv[i]);
 
   return ret;
+
+ bad:
+  for(i = 0; i < NELEM(argv) && argv[i] != 0; i++)
+    kfree(argv[i]);
+  return -1;
 }
 
 uint64
diff --git a/user/usertests.c b/user/usertests.c
index d2076c7..af3e5db 100644
--- a/user/usertests.c
+++ b/user/usertests.c
@@ -2016,6 +2016,7 @@ sbrkbugs(char *s)
 // has this bug, it will panic: balloc: out of blocks. 
 // assumed_free may need to be raised to be
 // more than the number of free blocks.
+// this test takes a long time.
 void
 badwrite(char *s)
 {
@@ -2048,6 +2049,22 @@ badwrite(char *s)
   exit(0);
 }
 
+// test whether exec() leaks memory if one of the
+// arguments is invalid. the test passes if
+// the kernel doesn't panic.
+void
+badarg(char *s)
+{
+  for(int i = 0; i < 50000; i++){
+    char *argv[2];
+    argv[0] = (char*)0xffffffff;
+    argv[1] = 0;
+    exec("echo", argv);
+  }
+  
+  exit(0);
+}
+
 // run each test in its own process. run returns 1 if child's exit()
 // indicates success.
 int
@@ -2087,7 +2104,8 @@ main(int argc, char *argv[])
   } tests[] = {
     {pgbug, "pgbug" },
     {sbrkbugs, "sbrkbugs" },
-    {badwrite, "badwrite" },
+    // {badwrite, "badwrite" },
+    {badarg, "badarg" },
     {reparent, "reparent" },
     {twochildren, "twochildren"},
     {forkfork, "forkfork"},