RFR: 8289778: ZGC: incorrect use of os::free() for mountpoint string handling after JDK-8289633

Jie Fu jiefu at openjdk.org
Wed Jul 6 02:24:13 UTC 2022


Hi all,

ZGC crashes were observed by us after JDK-8289633 due to incorrect use of `os::free()` for mountpoint string handling.

For example, `line_mountpoint` and `line_filesystem` will be allocated by `sscanf` @line60. 
And `line` will be allocated by `getline` @line84.


 53 char* ZMountPoint::get_mountpoint(const char* line, const char* filesystem) const {
 54   char* line_mountpoint = NULL;
 55   char* line_filesystem = NULL;
 56 
 57   // Parse line and return a newly allocated string containing the mount point if
 58   // the line contains a matching filesystem and the mount point is accessible by
 59   // the current user.
 60   if (sscanf(line, "%*u %*u %*u:%*u %*s %ms %*[^-]- %ms", &line_mountpoint, &line_filesystem) != 2 ||
 61       strcmp(line_filesystem, filesystem) != 0 ||
 62       access(line_mountpoint, R_OK|W_OK|X_OK) != 0) {
 63     // Not a matching or accessible filesystem
 64     os::free(line_mountpoint);
 65     line_mountpoint = NULL;
 66   }
 67 
 68   os::free(line_filesystem);
 69 
 70   return line_mountpoint;
 71 }
 72 
 73 void ZMountPoint::get_mountpoints(const char* filesystem, ZArray<char*>* mountpoints) const {
 74   FILE* fd = os::fopen(PROC_SELF_MOUNTINFO, "r");
 75   if (fd == NULL) {
 76     ZErrno err;
 77     log_error_p(gc)("Failed to open %s: %s", PROC_SELF_MOUNTINFO, err.to_string());
 78     return;
 79   }
 80 
 81   char* line = NULL;
 82   size_t length = 0;
 83 
 84   while (getline(&line, &length, fd) != -1) {
 85     char* const mountpoint = get_mountpoint(line, filesystem);
 86     if (mountpoint != NULL) {
 87       mountpoints->append(mountpoint);
 88     }
 89   }
 90 
 91   os::free(line);
 92   fclose(fd);
 93 }


See the anaylis of the crash reason in https://bugs.openjdk.org/browse/JDK-8289477

That means we have raw `::malloc() -> os::free()`, which is unbalanced.
Raw `::malloc()` does not write the header `os::free()` expects.
If NMT is on, we assert now, because NMT does not find its header in os::free().


The fix just reverts `os::free()` to `::free()`.

Testing:
  - hotspot/jtreg/gc/z on Linux/x64, all passed

Thanks.
Best regards,
Jie

-------------

Commit messages:
 - 8289778: ZGC: incorrect use of os::free() for mountpoint string handling after JDK-8289633

Changes: https://git.openjdk.org/jdk/pull/9387/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9387&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8289778
  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/9387.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9387/head:pull/9387

PR: https://git.openjdk.org/jdk/pull/9387


More information about the hotspot-dev mailing list