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

[Thomas Stuefe]

On Thu, 7 Jul 2022 09:29:12 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> 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 comment

Thanks!

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

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