Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in byterun/startup.c #7958

Open
vicuna opened this Issue Nov 25, 2002 · 1 comment

Comments

Projects
None yet
1 participant
@vicuna
Copy link
Collaborator

vicuna commented Nov 25, 2002

Original bug ID: 1482
Reporter: administrator
Status: acknowledged
Resolution: open
Priority: normal
Severity: feature
Category: runtime system and C interface
Tags: patch

Bug description

Full_Name: Blair Zajac
Version: 3.06
OS: Linux
Submission from: bdsl.66.12.233.174.gte.net (66.12.233.174)

In the ocaml-3.06 build directory after building everything, running

% valgrind --leak-check=yes boot/ocamlrun ocaml < /dev/null
...
        Objective Caml version 3.06

#
==9689==
==9689== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==9689== malloc/free: in use at exit: 1530198 bytes in 25 blocks.
==9689== malloc/free: 68 allocs, 43 frees, 1691808 bytes allocated.
==9689== For counts of detected errors, rerun with: -v
==9689== searching for pointers to 25 not-freed blocks.
==9689== checked 5741940 bytes.
==9689==
==9689== definitely lost: 52 bytes in 1 blocks.
==9689== possibly lost:   0 bytes in 0 blocks.
==9689== still reachable: 1530146 bytes in 24 blocks.
==9689==
==9689== 52 bytes in 1 blocks are definitely lost in loss record 1 of 5
==9689==    at 0x4006034B: malloc (vg_clientfuncs.c:100)
==9689==    by 0x804F175: stat_alloc (memory.c:338)
==9689==    by 0x805E12C: search_in_path (unix.c:78)
==9689==    by 0x805E19F: search_exe_in_path (unix.c:138)
==9689==
==9689== LEAK SUMMARY:
==9689==    definitely lost: 52 bytes in 1 blocks.
==9689==    possibly lost:   0 bytes in 0 blocks.
==9689==    still reachable: 1530146 bytes in 24 blocks.
==9689== Reachable blocks (those to which a pointer was found) are not shown.
==9689== To see them, rerun with: --show-reachable=yes
==9689==

This fix is a one line addition to byterun/startup.c. I'm attaching a patch
that modifies a couple of things:

  1. Fix the memory leak in exe_name.
  2. Include limits.h to get PATH_MAX. Use it to give proc_self_exe enough space
    for an arbitrary path.
  3. Check the result of lseek in read_trailer.
  4. Add a comment in a case statement to make it clear that failling through
    without doing anything is the expected behavior.
diff -x CVS -ru ../ocaml-3.06-no-compile/byterun/startup.c ./byterun/startup.c
--- ../ocaml-3.06-no-compile/byterun/startup.c  2002-06-05 05:26:08.000000000
-0700
+++ ./byterun/startup.c 2002-11-25 10:34:17.000000000 -0800
@@ -19,6 +19,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
+#include <limits.h>
 #include "config.h"
 #ifdef HAS_UNISTD
 #include <unistd.h>
@@ -84,11 +85,12 @@

 static int read_trailer(int fd, struct exec_trailer *trail)
 {
-  lseek(fd, (long) -TRAILER_SIZE, SEEK_END);
+  if (lseek(fd, (long) -TRAILER_SIZE, SEEK_END) == (off_t)-1)
+    return BAD_BYTECODE;
   if (read(fd, (char *) trail, TRAILER_SIZE) < TRAILER_SIZE)
     return BAD_BYTECODE;
   fixup_endianness_trailer(&trail->num_sections);
-  if (strncmp(trail->magic, EXEC_MAGIC, 12) == 0)
+  if (strncmp(trail->magic, EXEC_MAGIC, sizeof(trail->magic)) == 0)
     return 0;
   else
     return BAD_BYTECODE;
@@ -321,7 +323,7 @@
   char * shared_lib_path, * shared_libs, * req_prims;
   char * exe_name;
 #ifdef __linux__
-  static char proc_self_exe[256];
+  static char proc_self_exe[PATH_MAX+1];
   int retcode;
 #endif

@@ -351,6 +353,7 @@
     pos = parse_command_line(argv);
     if (argv[pos] == 0)
       fatal_error("No bytecode file specified.\n");
+    stat_free(exe_name);
     exe_name = argv[pos];
     fd = attempt_open(&exe_name, &trail, 1);
     switch(fd) {
@@ -362,6 +365,10 @@
         "Fatal error: the file %s is not a bytecode executable file\n",
         argv[pos]);
       break;
+    default:
+      /* There were no errors in opening and checking the bytecode
+       * file for its magic number, so fall through and use it. */
+      break;
     }
   }
   /* Read the table of contents (section descriptors) */
@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 7, 2016

Comment author: @mshinwell

Related to #71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.