6811367: Fix code in HeapDumper::dump_heap() to avoid buffer overrun

Fri, 14 Jan 2011 13:47:53 -0500

author
coleenp
date
Fri, 14 Jan 2011 13:47:53 -0500
changeset 2463
17c778814856
parent 2462
8012aa3ccede
child 2464
633a44a9fc45

6811367: Fix code in HeapDumper::dump_heap() to avoid buffer overrun
Summary: Check buffer size before using and use dynamic buffer sizes for subsequent calls.
Reviewed-by: kamg, dholmes

src/share/vm/services/heapDumper.cpp file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/services/heapDumper.cpp	Thu Jan 13 22:15:41 2011 -0800
     1.2 +++ b/src/share/vm/services/heapDumper.cpp	Fri Jan 14 13:47:53 2011 -0500
     1.3 @@ -453,7 +453,7 @@
     1.4  
     1.5  DumpWriter::~DumpWriter() {
     1.6    // flush and close dump file
     1.7 -  if (file_descriptor() >= 0) {
     1.8 +  if (is_open()) {
     1.9      close();
    1.10    }
    1.11    if (_buffer != NULL) os::free(_buffer);
    1.12 @@ -463,9 +463,10 @@
    1.13  // closes dump file (if open)
    1.14  void DumpWriter::close() {
    1.15    // flush and close dump file
    1.16 -  if (file_descriptor() >= 0) {
    1.17 +  if (is_open()) {
    1.18      flush();
    1.19      ::close(file_descriptor());
    1.20 +    set_file_descriptor(-1);
    1.21    }
    1.22  }
    1.23  
    1.24 @@ -1935,18 +1936,32 @@
    1.25  void HeapDumper::dump_heap(bool oome) {
    1.26    static char base_path[JVM_MAXPATHLEN] = {'\0'};
    1.27    static uint dump_file_seq = 0;
    1.28 -  char   my_path[JVM_MAXPATHLEN] = {'\0'};
    1.29 +  char* my_path;
    1.30 +  const int max_digit_chars = 20;
    1.31 +
    1.32 +  const char* dump_file_name = "java_pid";
    1.33 +  const char* dump_file_ext  = ".hprof";
    1.34  
    1.35    // The dump file defaults to java_pid<pid>.hprof in the current working
    1.36    // directory. HeapDumpPath=<file> can be used to specify an alternative
    1.37    // dump file name or a directory where dump file is created.
    1.38    if (dump_file_seq == 0) { // first time in, we initialize base_path
    1.39 +    // Calculate potentially longest base path and check if we have enough
    1.40 +    // allocated statically.
    1.41 +    const size_t total_length =
    1.42 +                      (HeapDumpPath == NULL ? 0 : strlen(HeapDumpPath)) +
    1.43 +                      strlen(os::file_separator()) + max_digit_chars +
    1.44 +                      strlen(dump_file_name) + strlen(dump_file_ext) + 1;
    1.45 +    if (total_length > sizeof(base_path)) {
    1.46 +      warning("Cannot create heap dump file.  HeapDumpPath is too long.");
    1.47 +      return;
    1.48 +    }
    1.49 +
    1.50      bool use_default_filename = true;
    1.51      if (HeapDumpPath == NULL || HeapDumpPath[0] == '\0') {
    1.52        // HeapDumpPath=<file> not specified
    1.53      } else {
    1.54 -      assert(strlen(HeapDumpPath) < sizeof(base_path), "HeapDumpPath too long");
    1.55 -      strcpy(base_path, HeapDumpPath);
    1.56 +      strncpy(base_path, HeapDumpPath, sizeof(base_path));
    1.57        // check if the path is a directory (must exist)
    1.58        DIR* dir = os::opendir(base_path);
    1.59        if (dir == NULL) {
    1.60 @@ -1960,8 +1975,6 @@
    1.61            char* end = base_path;
    1.62            end += (strlen(base_path) - fs_len);
    1.63            if (strcmp(end, os::file_separator()) != 0) {
    1.64 -            assert(strlen(base_path) + strlen(os::file_separator()) < sizeof(base_path),
    1.65 -              "HeapDumpPath too long");
    1.66              strcat(base_path, os::file_separator());
    1.67            }
    1.68          }
    1.69 @@ -1969,21 +1982,26 @@
    1.70      }
    1.71      // If HeapDumpPath wasn't a file name then we append the default name
    1.72      if (use_default_filename) {
    1.73 -      char fn[32];
    1.74 -      sprintf(fn, "java_pid%d", os::current_process_id());
    1.75 -      assert(strlen(base_path) + strlen(fn) + strlen(".hprof") < sizeof(base_path), "HeapDumpPath too long");
    1.76 -      strcat(base_path, fn);
    1.77 -      strcat(base_path, ".hprof");
    1.78 +      const size_t dlen = strlen(base_path);  // if heap dump dir specified
    1.79 +      jio_snprintf(&base_path[dlen], sizeof(base_path)-dlen, "%s%d%s",
    1.80 +                   dump_file_name, os::current_process_id(), dump_file_ext);
    1.81      }
    1.82 -    assert(strlen(base_path) < sizeof(my_path), "Buffer too small");
    1.83 -    strcpy(my_path, base_path);
    1.84 +    const size_t len = strlen(base_path) + 1;
    1.85 +    my_path = (char*)os::malloc(len);
    1.86 +    if (my_path == NULL) {
    1.87 +      warning("Cannot create heap dump file.  Out of system memory.");
    1.88 +      return;
    1.89 +    }
    1.90 +    strncpy(my_path, base_path, len);
    1.91    } else {
    1.92      // Append a sequence number id for dumps following the first
    1.93 -    char fn[33];
    1.94 -    sprintf(fn, ".%d", dump_file_seq);
    1.95 -    assert(strlen(base_path) + strlen(fn) < sizeof(my_path), "HeapDumpPath too long");
    1.96 -    strcpy(my_path, base_path);
    1.97 -    strcat(my_path, fn);
    1.98 +    const size_t len = strlen(base_path) + max_digit_chars + 2; // for '.' and \0
    1.99 +    my_path = (char*)os::malloc(len);
   1.100 +    if (my_path == NULL) {
   1.101 +      warning("Cannot create heap dump file.  Out of system memory.");
   1.102 +      return;
   1.103 +    }
   1.104 +    jio_snprintf(my_path, len, "%s.%d", base_path, dump_file_seq);
   1.105    }
   1.106    dump_file_seq++;   // increment seq number for next time we dump
   1.107  
   1.108 @@ -1991,4 +2009,5 @@
   1.109                      true  /* send to tty */,
   1.110                      oome  /* pass along out-of-memory-error flag */);
   1.111    dumper.dump(my_path);
   1.112 +  os::free(my_path);
   1.113  }

mercurial