[MDEV-24053] MSAN use-of-uninitialized-value in tpool::simulated_aio::simulated_aio_callback() Created: 2020-10-29  Updated: 2020-10-29  Resolved: 2020-10-29

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.5
Fix Version/s: 10.5.7

Type: Bug Priority: Major
Reporter: Marko Mäkelä Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: MSAN, regression

Issue Links:
Problem/Incident
is caused by MDEV-23855 InnoDB log checkpointing causes regre... Closed

 Description   

A change in MDEV-23855 introduced a regression for MemorySanitizer: the doublewrite buffer would write uninitialized data to the file.

I did not find out why exactly that commit introduced the failure, but the fix is simple:

diff --git a/storage/innobase/buf/buf0dblwr.cc b/storage/innobase/buf/buf0dblwr.cc
 
index a8ae3e2782a..c14634ae229 100644
 
--- a/storage/innobase/buf/buf0dblwr.cc
 
+++ b/storage/innobase/buf/buf0dblwr.cc
 
@@ -736,7 +736,8 @@ void buf_dblwr_t::add_to_batch(const IORequest &request, size_t size)
 
   encryption and/or page compression */
 
   void *frame= buf_page_get_frame(request.bpage);
 
 
-  memcpy_aligned<OS_FILE_LOG_BLOCK_SIZE>(p, frame, size);
 
+  memcpy_aligned<256>(p, frame, size); /* fil_page_compress() guarantee */
 
+  memset_aligned<256>(p + size, 0, srv_page_size - size);
 
   ut_ad(!request.bpage->zip_size() || request.bpage->zip_size() == size);
 
   ut_ad(active_slot->reserved == active_slot->first_free);
 
   ut_ad(active_slot->reserved < buf_size);

This is also correcting the alignment assertion. fil_page_compress() for page_compressed pages is only guaranteeing 256-byte alignment, not 512 bytes. For ROW_FORMAT=COMPRESSED pages, the minimum alignment is 1024 bytes.

 Comments   
Comment by Marko Mäkelä [ 2020-10-29 ]

It turns out that the alignment assumptions in memcpy_aligned and memset_aligned do not cover the size, but only the pointers. Hence, the 512-byte alignment of the existing call is not incorrect, but only inaccurate. I would propose an improved patch:

diff --git a/storage/innobase/buf/buf0dblwr.cc b/storage/innobase/buf/buf0dblwr.cc
 
index a8ae3e2782a..284936ddc43 100644
 
--- a/storage/innobase/buf/buf0dblwr.cc
 
+++ b/storage/innobase/buf/buf0dblwr.cc
 
@@ -736,7 +736,11 @@ void buf_dblwr_t::add_to_batch(const IORequest &request, size_t size)
 
   encryption and/or page compression */
 
   void *frame= buf_page_get_frame(request.bpage);
 
 
-  memcpy_aligned<OS_FILE_LOG_BLOCK_SIZE>(p, frame, size);
 
+  /* "frame" is at least 1024-byte aligned for ROW_FORMAT=COMPRESSED pages,
 
+  and at least srv_page_size (4096-byte) for everything else. */
 
+  memcpy_aligned<UNIV_ZIP_SIZE_MIN>(p, frame, size);
 
+  /* fil_page_compress() for page_compressed guarantees 256-byte alignment */
 
+  memset_aligned<256>(p + size, 0, srv_page_size - size);
 
   ut_ad(!request.bpage->zip_size() || request.bpage->zip_size() == size);
 
   ut_ad(active_slot->reserved == active_slot->first_free);
 
   ut_ad(active_slot->reserved < buf_size);

Generated at Thu Feb 08 09:27:05 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.