Bug #99078 A memory leak might happen when doing lf_hash_insert
Submitted: 26 Mar 2020 10:41 Modified: 9 Apr 2020 18:53
Reporter: Xiong Wang Email Updates:
Status: Closed Impact on me:
Category:MySQL Server: Performance Schema Severity:S2 (Serious)
Version:8.0 OS:Any
Assigned to: CPU Architecture:Any

[26 Mar 2020 10:41] Xiong Wang
From the source, a memory leak might happen in function lf_hash_insert.
int lf_hash_insert(LF_HASH *hash, LF_PINS *pins, const void *data) {
  int csize, bucket, hashnr;
  LF_SLIST *node;
  std::atomic<LF_SLIST *> *el;

  node = (LF_SLIST *)lf_alloc_new(pins);
  if (unlikely(!node)) {
    return -1;
==================> Until here, node has been created.
  uchar *extra_data =
      (uchar *)(node + 1);  // Stored immediately after the node.
  if (hash->initialize) {
    (*hash->initialize)(extra_data, (const uchar *)data);
  } else {
    memcpy(extra_data, data, hash->element_size);
  node->key = hash_key(hash, (uchar *)(node + 1), &node->keylen);
  hashnr = calc_hash(hash, node->key, node->keylen);
  bucket = hashnr % hash->size;
  el = static_cast<std::atomic<LF_SLIST *> *>(
      lf_dynarray_lvalue(&hash->array, bucket));
  if (unlikely(!el)) {
===================> Error happens, but the created 'node'  isn't freed
    return -1;
  if (el->load() == nullptr &&
      unlikely(initialize_bucket(hash, el, bucket, pins))) {
===================> Ditto
    return -1;
  node->hashnr = my_reverse_bits(hashnr) | 1; /* normal node */
  if (linsert(el, hash->charset, node, pins, hash->flags)) {
    lf_pinbox_free(pins, node); =============> Here, the node is freed.
    return 1;

How to repeat:
Look through source code

Suggested fix:
When error happens, lf_pinbox_free should be called at the same time.
[27 Mar 2020 13:33] MySQL Verification Team
Hello Mr. Wang,

Thank you for your bug report.

I have carefully analysed that function and entire code where this function is utilised.

I must admit that you are correct in your analysis. Not only that the pointer in question is allocated from heap (and not from stack), but also there are many places in the source code files where you have a conditional statement like this:

If (lf_hash_insert(.......))

although this function can return both -1 or 1 in case of error. I have not encountered (so far) a single place where those two error codes are distinguished.

Verified as reported, but with a higher severity.
[9 Apr 2020 18:53] Paul DuBois
Posted by developer:
Fixed in 8.0.21.

A potential memory leak in lf_hash_insert() was fixed.
[10 Apr 2020 12:26] MySQL Verification Team
Thank you, Paul.