Skip to content

MDEV-39240 10.6-11.4 Replication Allows Full Range for 32-bit Unsigned Timestamps#4933

Open
ParadoxV5 wants to merge 1 commit into10.11from
MDEV-39240
Open

MDEV-39240 10.6-11.4 Replication Allows Full Range for 32-bit Unsigned Timestamps#4933
ParadoxV5 wants to merge 1 commit into10.11from
MDEV-39240

Conversation

@ParadoxV5
Copy link
Copy Markdown
Contributor

Row-based Replication did not validate serialized timestamps, which allows (pre-11.5) replicas to accept “negative” timestamps (between the Year 2038 limit and the 4-byte limit) via replication. In particular, a completely valid source of those timestamps is 11.5.1+ primaries, where MDEV-32188 has increased the upper limit of timestamps to Year 2106 by utilizing this previously invalid range.

This commit fills in this pre-processing – swap the Year 2106 max with the Year 2038 max, or error on the others.

…d Timestamps

Row-based Replication did not vaidate serialized timestamps,
which allows (pre-11.5) replicas to accept “negative” timestamps
(between the Year 2038 limit and the 4-byte limit) via replication.
In particular, a completely valid source of those timestamps is 11.5.1+
primaries, where MDEV-32188 has increased the upper limit of timestamps
to Year 2106 by utilizing this previously invalid range.

This commits fills in this pre-processing –
swap the Year 2106 max with the Year 2038 max or error on the others.
@ParadoxV5 ParadoxV5 requested a review from bnestere April 14, 2026 03:18
@ParadoxV5 ParadoxV5 added MariaDB Corporation Replication Patches involved in replication labels Apr 14, 2026
Copy link
Copy Markdown
Contributor Author

@ParadoxV5 ParadoxV5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is based on Community Server 10.11 as CS 10.6 support ends soon.
Enterprise Server 10.6 will also receive this fix.
Let me know if CS 10.6 should also receive this patch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we only need to cover Row-based Replication here, since changing Statement-based Replication requires a mode (be it @@sql_mode or replication-exclusive) that normalizes Y2106 in multiple sources of timestamp values.

DBUG_RETURN(HA_ERR_CORRUPT_EVENT);
}

// Validate this external data
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use rli->relay_log.description_event_for_exec->server_version_split (which rpl_master_has_bug() uses) to decide what to do with the new unsigned range, but I believe they need to be validated for any primary’s version like any external input.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the discussion on Zoom, this should error even with @@slave_type_conversions=ALL_LOSSY enabled (instead of clamping to Y2038), because ‘lossy’ is intended more for “(decimal) precision” than “range maximum”.
I still put the test in @@slave_type_conversions’s script, not only to utilize suite/rpl/include/check_type.inc, but also to assert that @@slave_type_conversions doesn’t change the treatment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


However, variable substitution for SQL commands can’t work with strings containing both ' and ", which affects suite/rpl/include/check_type.inc (and others, such as include/write_var_to_file.inc), so I switched to Last_SQL_Errno instead.
(Help thread: Escaping quotes for --eval in MTR)

Comment on lines +108 to +111
DBUG_EXECUTE_IF("rpl_pack_simulate_negation",
for (uchar *byte= old_pack_ptr; byte < pack_ptr; ++byte)
*byte= ~*byte;
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered sneaking over-max timestamps into the primary server, but I’m concerned that it’s too UB from reading the relevant entry code, and so decided not to fix any tangential problems this hack surfaces.

{
if (likely(seconds >= 0 && seconds <= TIMESTAMP_MAX_VALUE))
break;
else if (likely(seconds == UINT_MAX32)) // They are both signed.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_time_t and UINT_MAX32 are both signed before MDEV-32188 (11.5.1).
(-1’s equivalence is only de facto.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Corporation Replication Patches involved in replication

Development

Successfully merging this pull request may close these issues.

1 participant