Skip to content

Commit

Permalink
MDEV-26272: The macro MASTER_INFO_VAR invokes undefined behaviour
Browse files Browse the repository at this point in the history
Updates to specific replication system variables need to target the
active primary connection to support multi-source replication. These
variables use the Sys_var_multi_source_ulonglong type. This class
uses offsets of the Master_info C++ class to generalize access to
its member variables.

The problem is that the Master_info class is not of standard layout,
and neither are many of its member variables, e.g. rli and
rli->relay_log. Because the class is not of standard layout, using
offsets to access member variables invokes undefined behavior.

This patch changes how Sys_var_multi_source_ulonglong accesses the
member variables of Master_info from using parameterized memory
offsets to “getter” function pointers.

Note that the size parameter and assertion are removed, as they are
no longer needed because the condition is guaranteed by compiler
type-safety checks.

Reviewed By:
============
Kristian Nielsen <knielsen@knielsen-hq.org>
  • Loading branch information
bnestere committed Oct 30, 2023
1 parent eb8053b commit e52777f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
10 changes: 10 additions & 0 deletions sql/rpl_mi.h
Expand Up @@ -210,6 +210,16 @@ class Master_info : public Slave_reporting_capability
void lock_slave_threads();
void unlock_slave_threads();

ulonglong get_slave_skip_counter()
{
return rli.slave_skip_counter;
}

ulonglong get_max_relay_log_size()
{
return rli.max_relay_log_size;
}

/* the variables below are needed because we can change masters on the fly */
char master_log_name[FN_REFLEN+6]; /* Room for multi-*/
char host[HOSTNAME_LENGTH*SYSTEM_CHARSET_MBMAXLEN+1];
Expand Down
8 changes: 4 additions & 4 deletions sql/sys_vars.cc
Expand Up @@ -5199,15 +5199,15 @@ static Sys_var_uint Sys_slave_net_timeout(
*/

ulonglong Sys_var_multi_source_ulonglong::
get_master_info_ulonglong_value(THD *thd, ptrdiff_t offset) const
get_master_info_ulonglong_value(THD *thd) const
{
Master_info *mi;
ulonglong res= 0; // Default value
mysql_mutex_unlock(&LOCK_global_system_variables);
if ((mi= get_master_info(&thd->variables.default_master_connection,
Sql_condition::WARN_LEVEL_WARN)))
{
res= *((ulonglong*) (((uchar*) mi) + master_info_offset));
res= (mi->*mi_accessor_func)();
mi->release();
}
mysql_mutex_lock(&LOCK_global_system_variables);
Expand Down Expand Up @@ -5277,7 +5277,7 @@ static bool update_slave_skip_counter(sys_var *self, THD *thd, Master_info *mi)
static Sys_var_multi_source_ulonglong Sys_slave_skip_counter(
"sql_slave_skip_counter", "Skip the next N events from the master log",
SESSION_VAR(slave_skip_counter), NO_CMD_LINE,
MASTER_INFO_VAR(rli.slave_skip_counter),
&Master_info::get_slave_skip_counter,
VALID_RANGE(0, UINT_MAX), DEFAULT(0), BLOCK_SIZE(1),
ON_UPDATE(update_slave_skip_counter));

Expand All @@ -5293,7 +5293,7 @@ static Sys_var_multi_source_ulonglong Sys_max_relay_log_size(
"relay log will be rotated automatically when the size exceeds this "
"value. If 0 at startup, it's set to max_binlog_size",
SESSION_VAR(max_relay_log_size), CMD_LINE(REQUIRED_ARG),
MASTER_INFO_VAR(rli.max_relay_log_size),
&Master_info::get_max_relay_log_size,
VALID_RANGE(0, 1024L*1024*1024), DEFAULT(0), BLOCK_SIZE(IO_SIZE),
ON_UPDATE(update_max_relay_log_size));

Expand Down
17 changes: 7 additions & 10 deletions sql/sys_vars.inl
Expand Up @@ -2322,10 +2322,10 @@ public:
like sql_slave_skip_counter are GLOBAL.
*/

#define MASTER_INFO_VAR(X) my_offsetof(Master_info, X), sizeof(((Master_info *)0x10)->X)
class Sys_var_multi_source_ulonglong;
class Master_info;

typedef ulonglong (Master_info::*mi_ulonglong_accessor_function)(void);
typedef bool (*on_multi_source_update_function)(sys_var *self, THD *thd,
Master_info *mi);
bool update_multi_source_variable(sys_var *self,
Expand All @@ -2334,26 +2334,23 @@ bool update_multi_source_variable(sys_var *self,

class Sys_var_multi_source_ulonglong :public Sys_var_ulonglong
{
ptrdiff_t master_info_offset;
mi_ulonglong_accessor_function mi_accessor_func;
on_multi_source_update_function update_multi_source_variable_func;
public:
Sys_var_multi_source_ulonglong(const char *name_arg,
const char *comment, int flag_args,
ptrdiff_t off, size_t size,
CMD_LINE getopt,
ptrdiff_t master_info_offset_arg,
size_t master_info_arg_size,
mi_ulonglong_accessor_function mi_accessor_arg,
ulonglong min_val, ulonglong max_val,
ulonglong def_val, uint block_size,
on_multi_source_update_function on_update_func)
:Sys_var_ulonglong(name_arg, comment, flag_args, off, size,
getopt, min_val, max_val, def_val, block_size,
0, VARIABLE_NOT_IN_BINLOG, 0, update_multi_source_variable),
master_info_offset(master_info_offset_arg),
mi_accessor_func(mi_accessor_arg),
update_multi_source_variable_func(on_update_func)
{
SYSVAR_ASSERT(master_info_arg_size == size);
}
{ }
bool global_update(THD *thd, set_var *var)
{
return session_update(thd, var);
Expand All @@ -2367,15 +2364,15 @@ public:
{
ulonglong *tmp, res;
tmp= (ulonglong*) (((uchar*)&(thd->variables)) + offset);
res= get_master_info_ulonglong_value(thd, master_info_offset);
res= get_master_info_ulonglong_value(thd);
*tmp= res;
return (uchar*) tmp;
}
const uchar *global_value_ptr(THD *thd, const LEX_CSTRING *base) const
{
return session_value_ptr(thd, base);
}
ulonglong get_master_info_ulonglong_value(THD *thd, ptrdiff_t offset) const;
ulonglong get_master_info_ulonglong_value(THD *thd) const;
bool update_variable(THD *thd, Master_info *mi)
{
return update_multi_source_variable_func(this, thd, mi);
Expand Down

0 comments on commit e52777f

Please sign in to comment.