Hi Alexey, Thanks for the patch. Please see below my review comments.
The stack smashing is still present in x86 builders. This one https://buildbot.mariadb.org/#/builders/862/builds/7426 is quite revealing: > #13 0xf0515578 in spider_db_append_xid_str (tmp_str=0xf39af80c, > xid=0xef1796e8) at > /home/buildbot/x86-debian-12-fulltest-debug/build/storage/spider/spd_db_conn.cc:1158 > format_id = "1234567" > format_id_length = 10 Note the relevant code: > void spider_db_append_xid_str( > spider_string *tmp_str, > XID *xid > ) { > char format_id[sizeof(long) + 3]; > uint format_id_length; > DBUG_ENTER("spider_db_append_xid_str"); > format_id_length = > my_sprintf(format_id, (format_id, "%lu", xid->formatID)); It suggests that long has size 4 on that builder, and so format_id is an array of length 7, thus string "1234567890" causes a stack smashing. The max length of an integer type in decimal representation is about 2.4 times sizeof the type. So simply adding a padding length of 3 is not gonna work. Please fix this. > From 18bfc285f5a29f381714b3b25f5461f688a6ff9a Mon Sep 17 00:00:00 2001 > From: Alexey Botchkov <[email protected]> > Date: Mon, 22 Dec 2025 00:46:10 +0400 > Subject: [PATCH] MDEV-35426 Stack smashing in spider_db_mbase::xa_end (opt), > ASAN: stack-buffer-overflow in Binary_string::q_append, and Assertion > `str.alloced_length() >= str.length() + data_len' failed in > spider_string::q_append on SELECT. > The buffer size has to be increased because we store the hexadecimal > representation of the data which takes twice it's length plust the '0x' > starter. > --- > .../mysql-test/spider/bugfix/r/xa_cmd.result | 9 +++++++++ > .../spider/mysql-test/spider/bugfix/t/xa_cmd.test | 10 ++++++++++ > storage/spider/spd_db_conn.cc | 2 -- > storage/spider/spd_db_conn.h | 2 ++ > storage/spider/spd_db_mysql.cc | 15 ++++++++++----- > 5 files changed, 31 insertions(+), 7 deletions(-) > diff --git a/storage/spider/mysql-test/spider/bugfix/r/xa_cmd.result > b/storage/spider/mysql-test/spider/bugfix/r/xa_cmd.result > index 5a560ce25d1..fb5dcc76f86 100644 > --- a/storage/spider/mysql-test/spider/bugfix/r/xa_cmd.result > +++ b/storage/spider/mysql-test/spider/bugfix/r/xa_cmd.result > @@ -34,6 +34,14 @@ INSERT INTO tbl_a (pkey) VALUES > (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); > XA END 'test'; > XA PREPARE 'test'; > XA COMMIT 'test'; > +# MDEV-35426 Stack smashing in spider_db_mbase::xa_end (opt), ASAN: stack- > +# buffer-overflow in Binary_string::q_append, and Assertion > +# `str.alloced_length() >= str.length() + data_len' failed in > +# spider_string::q_append on SELECT. > +XA START > 'xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc1','xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc2',1234567890; > +INSERT INTO tbl_a (pkey) VALUES (10); > +XA END > 'xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc1','xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc2',1234567890; > +XA ROLLBACK > 'xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc1','xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc2',1234567890; > # MDEV-37829 XA COMMIT ONE PHASE fails with "This xid does not > # exist" if a trigger references a SPIDER table > # > @@ -103,6 +111,7 @@ connection child2_1; > SELECT argument FROM mysql.general_log WHERE command_type != 'Execute' AND > argument LIKE '%insert %'; > argument > insert into > `auto_test_remote`.`tbl_a`(`pkey`)values(0),(1),(2),(3),(4),(5),(6),(7),(8),(9) > +insert into `auto_test_remote`.`tbl_a`(`pkey`)values(10) > insert into `auto_test_remote`.`tbl_a`(`pkey`)values(100) > insert high_priority into `auto_test_remote`.`tbl_a`(`pkey`)values(101) > insert high_priority into `auto_test_remote`.`tbl_a`(`pkey`)values(102) > diff --git a/storage/spider/mysql-test/spider/bugfix/t/xa_cmd.test > b/storage/spider/mysql-test/spider/bugfix/t/xa_cmd.test > index 5a44ab5e01b..b0116fdbeac 100644 > --- a/storage/spider/mysql-test/spider/bugfix/t/xa_cmd.test > +++ b/storage/spider/mysql-test/spider/bugfix/t/xa_cmd.test > @@ -48,6 +48,16 @@ XA END 'test'; > XA PREPARE 'test'; > XA COMMIT 'test'; > +--echo # MDEV-35426 Stack smashing in spider_db_mbase::xa_end (opt), ASAN: > stack- > +--echo # buffer-overflow in Binary_string::q_append, and Assertion > +--echo # `str.alloced_length() >= str.length() + data_len' failed in > +--echo # spider_string::q_append on SELECT. > + > +XA START > 'xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc1','xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc2',1234567890; > +INSERT INTO tbl_a (pkey) VALUES (10); > +XA END > 'xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc1','xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc2',1234567890; > +XA ROLLBACK > 'xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc1','xaaaaaaaaaaaaaaabbbbbbbbbbbbbbbccc2',1234567890; > + > --echo # MDEV-37829 XA COMMIT ONE PHASE fails with "This xid does not > --echo # exist" if a trigger references a SPIDER table > --echo # > diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc > index 79cafaa8f30..a544b761858 100644 > --- a/storage/spider/spd_db_conn.cc > +++ b/storage/spider/spd_db_conn.cc > @@ -48,8 +48,6 @@ extern SPIDER_DBTON spider_dbton[SPIDER_DBTON_SIZE]; > #define SPIDER_SQL_COALESCE_STR "coalesce(" > #define SPIDER_SQL_COALESCE_LEN (sizeof(SPIDER_SQL_COALESCE_STR) - 1) > -#define SPIDER_SQL_HEX_STR "0x" > -#define SPIDER_SQL_HEX_LEN (sizeof(SPIDER_SQL_HEX_STR) - 1) > #define SPIDER_SQL_SET_NAMES_STR "set names " > #define SPIDER_SQL_SET_NAMES_LEN sizeof(SPIDER_SQL_SET_NAMES_STR) - 1 > diff --git a/storage/spider/spd_db_conn.h b/storage/spider/spd_db_conn.h > index eb0a55e6288..e8448a75631 100644 > --- a/storage/spider/spd_db_conn.h > +++ b/storage/spider/spd_db_conn.h > @@ -24,6 +24,8 @@ > #define SPIDER_DB_KEY_NAME_LEN (sizeof(SPIDER_DB_KEY_NAME_STR) - 1) > #define SPIDER_DB_SEQUENCE_NAME_STR "" > #define SPIDER_DB_SEQUENCE_NAME_LEN (sizeof(SPIDER_DB_SEQUENCE_NAME_STR) - 1) > +#define SPIDER_SQL_HEX_STR "0x" > +#define SPIDER_SQL_HEX_LEN (sizeof(SPIDER_SQL_HEX_STR) - 1) > #define SPIDER_DB_TABLE_LOCK_READ_LOCAL 0 > #define SPIDER_DB_TABLE_LOCK_READ 1 > diff --git a/storage/spider/spd_db_mysql.cc b/storage/spider/spd_db_mysql.cc > index dce7573567a..cd7dca1d38a 100644 > --- a/storage/spider/spd_db_mysql.cc > +++ b/storage/spider/spd_db_mysql.cc > @@ -2596,7 +2596,8 @@ int spider_db_mbase::xa_end( > XID *xid, > int *need_mon > ) { > - char sql_buf[SPIDER_SQL_XA_END_LEN + XIDDATASIZE + sizeof(long) + 9]; > + char sql_buf[SPIDER_SQL_XA_END_LEN + XIDDATASIZE*2 + > + SPIDER_SQL_HEX_LEN*2 + sizeof(long) + 9]; Same problem with sizeof(long) here - please fix, and same below. > spider_string sql_str(sql_buf, sizeof(sql_buf), &my_charset_bin); > DBUG_ENTER("spider_db_mbase::xa_end"); > DBUG_PRINT("info",("spider this=%p", this)); > @@ -2613,7 +2614,8 @@ int spider_db_mbase::xa_prepare( > XID *xid, > int *need_mon > ) { > - char sql_buf[SPIDER_SQL_XA_PREPARE_LEN + XIDDATASIZE + sizeof(long) + 9]; > + char sql_buf[SPIDER_SQL_XA_PREPARE_LEN + XIDDATASIZE*2 + > + SPIDER_SQL_HEX_LEN*2 + sizeof(long) + 9]; > spider_string sql_str(sql_buf, sizeof(sql_buf), &my_charset_bin); > DBUG_ENTER("spider_db_mbase::xa_prepare"); > DBUG_PRINT("info",("spider this=%p", this)); > @@ -2630,7 +2632,8 @@ int spider_db_mbase::xa_commit( > XID *xid, > int *need_mon > ) { > - char sql_buf[SPIDER_SQL_XA_COMMIT_LEN + XIDDATASIZE + sizeof(long) + 9]; > + char sql_buf[SPIDER_SQL_XA_COMMIT_LEN + XIDDATASIZE*2 + > + SPIDER_SQL_HEX_LEN*2 + sizeof(long) + 9]; > spider_string sql_str(sql_buf, sizeof(sql_buf), &my_charset_bin); > DBUG_ENTER("spider_db_mbase::xa_commit"); > DBUG_PRINT("info",("spider this=%p", this)); > @@ -2647,7 +2650,8 @@ int spider_db_mbase::xa_rollback( > XID *xid, > int *need_mon > ) { > - char sql_buf[SPIDER_SQL_XA_ROLLBACK_LEN + XIDDATASIZE + sizeof(long) + 9]; > + char sql_buf[SPIDER_SQL_XA_ROLLBACK_LEN + XIDDATASIZE*2 + > + SPIDER_SQL_HEX_LEN*2 + sizeof(long) + 9]; > spider_string sql_str(sql_buf, sizeof(sql_buf), &my_charset_bin); > DBUG_ENTER("spider_db_mbase::xa_rollback"); > DBUG_PRINT("info",("spider this=%p", this)); > @@ -4754,7 +4758,8 @@ int spider_db_mbase_util::append_xa_start( > DBUG_ENTER("spider_db_mbase_util::append_xa_start"); > DBUG_PRINT("info",("spider this=%p", this)); > if (str->reserve(SPIDER_SQL_SEMICOLON_LEN + > - SPIDER_SQL_XA_START_LEN + XIDDATASIZE + sizeof(long) + 9)) > + SPIDER_SQL_XA_START_LEN + XIDDATASIZE*2 + SPIDER_SQL_HEX_LEN*2 + > + sizeof(long) + 9)) > DBUG_RETURN(HA_ERR_OUT_OF_MEM); > if (str->length()) > { > -- > 2.39.2 Best, Yuchen _______________________________________________ developers mailing list -- [email protected] To unsubscribe send an email to [email protected]
