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

Jie Fu jiefu at openjdk.org
Thu Jul 7 01:08:14 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

Jie Fu has updated the pull request incrementally with one additional commit since the last revision:

  Address review comments

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/9387/files
  - new: https://git.openjdk.org/jdk/pull/9387/files/c487491d..8ae3bfcf

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=9387&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9387&range=00-01

  Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 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