Skip to content

Memory leak in Linux::getdents64 #263

@tom91136

Description

@tom91136

@FinnWilkinson The function's malloc is missing the corresponding free:

result.d_name = (char*)malloc(result.d_namlen + 1);

Reading through the getdents64 implementation, the whole logic around populating this result.d_name field is a bit weird.

  • Some calls to memcpy is not qualified.
  • Computation of result.d_namlen creates a copy of next_direct->d_name as std::string only to call size() and discard the string.
  • Memory for that field is allocated but not freed, and this happens in a while loop where linux_dirent64 result does not escape.

Because dirent->d_name is guaranteed to be \0 terminated, we can just do this:

...
    linux_dirent64 result;
    result.d_ino = next_direct->d_ino;
#ifdef __MACH__
    result.d_off = next_direct->d_seekoff;
#else
    result.d_off = next_direct->d_off;
#endif
    std::string d_name = next_direct->d_name;
    result.d_type = next_direct->d_type;
    result.d_namlen = d_name.size();
    result.d_name = d_name.data();  // C++17's non-const overload, mutation of the returned char* is permitted
    // Get size of struct before alignment
    // 20 = combined size of d_ino, d_off, d_reclen, d_type, and d_name's
    // null-terminator
...

d_name's memory is now managed by RAII so all is sound.
Let me know if we want this version or just call free and I'll get a PR going.

PS: When we eventually switch to C++20, we probably want to rewrite the initialisation of linux_dirent64 to use designations.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions