Commit ae1d17f5 authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-20316 InnoDB writes uninitialised tail of XID buffer

Starting with commit 210855ce
Valgrind became aware that the unused tail of the buffer that
is returned by thd_get_xid() is actually uninitialized.

The problem should exist already in MySQL 5.0. I was able to
repeat it on MariaDB Server 5.5 with some additional instrumentation.
InnoDB is allocating 128+4+4 bytes for the XID and the lengths of
its components, even when the XID is shorter than 64+64 bytes.
In MariaDB Server 10.3, while running the test main.xa_binlog,
in the xid_t::set() that is called by sql_yacc.yy, the 128-byte data
buffer was uninitialized according to Valgrind, and only the first bytes
were initialized. When the xid_t::data was copied to
thd.transaction.xid_state.xid.data, it happened so that the entire
target buffer was considered initialized. With MariaDB Server 10.4 since
the said commit, Valgrind will correctly be detect the tail of the buffer
as uninitialized.

The impact of this bug is as follows:

(1) InnoDB will write unnecessarily much redo log for XA PREPARE.
(2) InnoDB will write garbage bytes to the redo log and undo log pages.
(3) The garbage should be 'harmless', because on recovery, only the
actual payload of the XID will be used, based on the written length.

trx_rseg_write_wsrep_checkpoint(), trx_undo_write_xid(): Write only
the actually used length of xid->data to the data page, and
zero out the rest of the buffer by mlog_memset().
parent 97bbac8e
......@@ -53,6 +53,10 @@ trx_rseg_write_wsrep_checkpoint(
const XID* xid,
mtr_t* mtr)
{
DBUG_ASSERT(xid->gtrid_length >= 0);
DBUG_ASSERT(xid->bqual_length >= 0);
DBUG_ASSERT(xid->gtrid_length + xid->bqual_length < XIDDATASIZE);
mlog_write_ulint(TRX_RSEG_WSREP_XID_FORMAT + rseg_header,
uint32_t(xid->formatID),
MLOG_4BYTES, mtr);
......@@ -65,9 +69,15 @@ trx_rseg_write_wsrep_checkpoint(
uint32_t(xid->bqual_length),
MLOG_4BYTES, mtr);
const ulint xid_length = static_cast<ulint>(xid->gtrid_length
+ xid->bqual_length);
mlog_write_string(TRX_RSEG_WSREP_XID_DATA + rseg_header,
reinterpret_cast<const byte*>(xid->data),
XIDDATASIZE, mtr);
xid_length, mtr);
if (UNIV_LIKELY(xid_length < XIDDATASIZE)) {
mlog_memset(TRX_RSEG_WSREP_XID_DATA + rseg_header + xid_length,
XIDDATASIZE - xid_length, 0, mtr);
}
}
/** Update the WSREP XID information in rollback segment header.
......
/*****************************************************************************
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2014, 2018, MariaDB Corporation.
Copyright (c) 2014, 2019, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
......@@ -662,6 +662,10 @@ trx_undo_write_xid(
const XID* xid, /*!< in: X/Open XA Transaction Identification */
mtr_t* mtr) /*!< in: mtr */
{
DBUG_ASSERT(xid->gtrid_length >= 0);
DBUG_ASSERT(xid->bqual_length >= 0);
DBUG_ASSERT(xid->gtrid_length + xid->bqual_length < XIDDATASIZE);
mlog_write_ulint(log_hdr + TRX_UNDO_XA_FORMAT,
static_cast<ulint>(xid->formatID),
MLOG_4BYTES, mtr);
......@@ -673,10 +677,15 @@ trx_undo_write_xid(
mlog_write_ulint(log_hdr + TRX_UNDO_XA_BQUAL_LEN,
static_cast<ulint>(xid->bqual_length),
MLOG_4BYTES, mtr);
const ulint xid_length = static_cast<ulint>(xid->gtrid_length
+ xid->bqual_length);
mlog_write_string(log_hdr + TRX_UNDO_XA_XID,
reinterpret_cast<const byte*>(xid->data),
XIDDATASIZE, mtr);
xid_length, mtr);
if (UNIV_LIKELY(xid_length < XIDDATASIZE)) {
mlog_memset(log_hdr + TRX_UNDO_XA_XID + xid_length,
XIDDATASIZE - xid_length, 0, mtr);
}
}
/********************************************************************//**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment