MDEV-36871 mariadb-backup incremental segfault querying mariadb_backup_history

[Thirunarayanan]

• The Jira issue number for this PR is: MDEV-36871

Description

Problem:

• Mariabackup tries to read the history data from mysql.mariadb_backup_history and fails with segfault. Reason is that mariabackup does force innodb_log_checkpoint_now from commit 652f33e(MDEV-30000).

• Mariabackup sends the "innodb_log_checkpoint_now=1" query to server and reads the result set for the query later in the code because the query may trigger the page thread to flush the pages. But before reading the query result for innodb_log_checkpoint_now=1, mariabackup does execute the select query for the history table (mysql.mariadb_backup_history) and wrongly reads the query result of innodb_log_checkpoint_now. This leads to assertion in mariabackup

Solution:

• Move the reading of history data from mysql.mariadb_backup_history after reading the result of innodb_log_checkpoint_now=1 query.

How can this PR be tested?

./mtr mariabackup.xbstream

Basing the PR against the correct MariaDB version

• This is a new feature or a refactoring, and the PR is based against the main branch.
• This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

• I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
• For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[vlad-lesin]

The code looks good to me. Please squash two commits into one and fix the commit header, it should correspond to Jira's ticket header.

I would also propose the following fix to avoid buffer overflow:

--- a/extra/mariabackup/backup_mysql.cc
+++ b/extra/mariabackup/backup_mysql.cc
@@ -610,12 +610,27 @@ select_incremental_lsn_from_history(lsn_t *incremental_lsn)
 {
 	MYSQL_RES *mysql_result;
 	char query[1000];
-	char buf[100];
+	char buf[NAME_LEN*2+3];
+
+	unsigned long opt_incremental_history_name_len=
+	  strlen(opt_incremental_history_name);
+	unsigned long opt_incremental_history_uuid_len=
+	  strlen(opt_incremental_history_uuid);
+	if (opt_incremental_history_name_len*2 > sizeof(buf))
+		die("Incremental history table name '%s' is too long.",
+		    opt_incremental_history_name);
+	if (opt_incremental_history_uuid_len*2 > sizeof(buf))
+		die("Incremental history uuid '%s' is too long.",
+		    opt_incremental_history_uuid);
+	if (opt_incremental_history_name && opt_incremental_history_name[0]
+	    && opt_incremental_history_uuid && opt_incremental_history_uuid[0])
+		die("It is allowed to use either --incremental-history-name "
+		    "or --incremental-history-uuid, but not both.");
 
 	if (opt_incremental_history_name) {
 		mysql_real_escape_string(mysql_connection, buf,
 				opt_incremental_history_name,
-				(unsigned long)strlen(opt_incremental_history_name));
+				opt_incremental_history_name_len);
 		snprintf(query, sizeof(query),
 			"SELECT innodb_to_lsn "
 			"FROM PERCONA_SCHEMA.xtrabackup_history "
@@ -624,11 +639,10 @@ select_incremental_lsn_from_history(lsn_t *incremental_lsn)
 			"ORDER BY innodb_to_lsn DESC LIMIT 1",
 			buf);
 	}
-
-	if (opt_incremental_history_uuid) {
+	else if (opt_incremental_history_uuid) {
 		mysql_real_escape_string(mysql_connection, buf,
 				opt_incremental_history_uuid,
-				(unsigned long)strlen(opt_incremental_history_uuid));
+				opt_incremental_history_uuid_len);
 		snprintf(query, sizeof(query),
 			"SELECT innodb_to_lsn "
 			"FROM PERCONA_SCHEMA.xtrabackup_history "
@@ -638,7 +652,9 @@ select_incremental_lsn_from_history(lsn_t *incremental_lsn)
 			buf);
 	}
 
-	mysql_result = xb_mysql_query(mysql_connection, query, true);
+	/* xb_mysql_query(..., ..., true) will die on error, so
+	mysql_result can't be nullptr */
+	mysql_result= xb_mysql_query(mysql_connection, query, true);
 
 	ut_ad(mysql_num_fields(mysql_result) == 1);
 	const MYSQL_ROW row = mysql_fetch_row(mysql_result);

I didn't check and tested the above code.