[MDEV-31450] mem_root->flags corruption in calculate_block_sizes Created: 2023-06-10  Updated: 2023-06-30  Resolved: 2023-06-30

Status: Closed
Project: MariaDB Server
Component/s: Server
Affects Version/s: 10.7, 10.8, 10.9, 10.10, 10.11, 11.1
Fix Version/s: 10.9.8, 10.11.5, 11.0.3

Type: Bug Priority: Minor
Reporter: Yury Chaikou Assignee: Michael Widenius
Resolution: Fixed Votes: 0
Labels: None


 Description   

&= changes mem_root->flags, which makes condition always true
Probably & should be used instead of &=

mysys/my_alloc.c

  if (mem_root->flags&= ROOT_FLAG_MPROTECT)         <<--
 
  {
 
    mem_root->block_size= MY_ALIGN(block_size, my_system_page_size);
 
    if (pre_alloc)
 
      pre_alloc= MY_ALIGN(pre_alloc, my_system_page_size);
 
  }



 Comments   
Comment by Daniel Black [ 2023-06-13 ]

Thanks Yury,

I've put you as the author of this fix on https://github.com/MariaDB/server/pull/2668.

Note, being &= it doesn't make it always true, but it does clear other flags like ROOT_FLAG_THREAD_SPECIFIC.

Comment by Yury Chaikou [ 2023-06-13 ]

Yes, that is right, it will not make the condition always true.
By the way, can we also change this
#define MALLOC_FLAG(A) ((A & 1) ? MY_THREAD_SPECIFIC : 0)
to
#define MALLOC_FLAG(A) ((A & MY_THREAD_SPECIFIC) ? MY_THREAD_SPECIFIC : 0)
to make it a bit safer?

Comment by Daniel Black [ 2023-06-13 ]

I'd be tempted to remove MALLOC_FLAG as its not actually used. But I'll leave that to Monty to decide in review.

Comment by Michael Widenius [ 2023-06-30 ]

This has already been fixed and pushed to 10.9
commit e1a631fecc364ce1268b13a8108a3186556d660c (HEAD -> 10.9, origin/bb-10.9-monty, origin/10.9)
Author: Monty <monty@mariadb.org>
Date: Mon Jun 12 20:11:32 2023 +0300
Fixed wrong assignment in calculate_block_sizes() for MEM_ROOT

The effect was that that ROOT_FLAG_THREAD_SPECIFIC was cleared and
the memory allocated by memroot would be contributed the the system,
not to the thread.

This exposed a bug in how "show explain for ..." allocated data.

  • The thread that did provide the explain allocated data in the
    "show explain" threads mem_root, which is marked as THREAD_SPECIFIC.
  • Fixed by allocating the explain data in a temporary explain_mem_root
    which is not THREAD_SPECIFIC.

Other things:

  • Added extra checks when using update_malloc_size()
  • Do not call update_malloc_size() for memory not registered with
    update_malloc_size(). This avoid some wrong 'memory not freed' reports.
  • Added a checking of 'thd->killed' to ensure that
    main.truncate_notembedded.test still works.

Reported by: Yury Chaikou

Comment by Michael Widenius [ 2023-06-30 ]

Already fixed

Generated at Thu Feb 08 10:23:58 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.