MDEV-39556 SIGSEGV in ha_mroonga::storage_set_keys_in_use on SELECT#5213
MDEV-39556 SIGSEGV in ha_mroonga::storage_set_keys_in_use on SELECT#5213grooverdan wants to merge 1 commit into
Conversation
Many pathes exist for Mroonga to open tables without opening indexes. Altering from another storage engine and accessing the information schema statistics is one mechanism. The fix is a backport from upstream. Backport from mroonga/mroonga@0b35419#diff-c026dc94c7f79d426e7a68e1e69fdc0f7743296a37d6cd19c9e7117a763f242e Fix a crash bug that may be caused after MySQL/MariaDB upgrade GitHub: fix MariaDBGH-423 MySQL/MariaDB may open Mroonga tables by "CHECK TABLES" when MySQL/MariaDB is upgraded. Mroonga didn't open indexes when a table is opened by "CHECK TABLES". If "REPAIR TABLE" is needed, a table is reopened. If "REPAIR TABLE" isn't needed, a table is NOT reopened. The table doesn't open indexes. "SELECT ... MATCH AGAINST" for the table causes a crash. Reported by Vincent Pelletier. Thanks!!! Backported to MariaDB by Daniel Black
There was a problem hiding this comment.
Code Review
This pull request refactors the table and index opening logic in Mroonga to handle repair operations more robustly, and adds a null check for grn_index_tables in storage_set_keys_in_use to fix a SIGSEGV crash (MDEV-39556). The review feedback highlights two critical issues in the new error-handling flow: first, in wrapper_open, calling wrapper_open_indexes after a failed open_table is unsafe and could lead to a crash; second, in storage_open, a tolerated failure in storage_open_indexes during a repair operation does not reset the error variable, which may incorrectly cause the overall open operation to fail.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| error = open_table(name); | ||
| if (error && !(open_options & HA_OPEN_FOR_REPAIR)) | ||
| goto error_exit; | ||
|
|
||
| error = wrapper_open_indexes(name); | ||
| if (error) { | ||
| grn_obj_unlink(ctx, grn_table); | ||
| grn_table = NULL; | ||
| DBUG_RETURN(error); | ||
| } | ||
| } | ||
| error = wrapper_open_indexes(name); | ||
| if (error && !(open_options & HA_OPEN_FOR_REPAIR)) | ||
| goto error_exit; | ||
|
|
||
| error = 0; |
There was a problem hiding this comment.
If open_table(name) fails and HA_OPEN_FOR_REPAIR is set, error will be non-zero, but the execution will continue instead of jumping to error_exit. In this scenario, calling wrapper_open_indexes(name) is unsafe because the table was not successfully opened (meaning grn_table is likely NULL or invalid), which can lead to a crash (segmentation fault) inside wrapper_open_indexes.
We should only attempt to open the indexes if open_table succeeded. If open_table failed but HA_OPEN_FOR_REPAIR is set, we should skip opening the indexes and proceed.
error = open_table(name);
if (error) {
if (!(open_options & HA_OPEN_FOR_REPAIR))
goto error_exit;
} else {
error = wrapper_open_indexes(name);
if (error && !(open_options & HA_OPEN_FOR_REPAIR))
goto error_exit;
}
error = 0;| error = storage_open_indexes(name); | ||
| if (error && !(open_options & HA_OPEN_FOR_REPAIR)) { | ||
| storage_close_columns(); | ||
| grn_obj_unlink(ctx, grn_table); | ||
| grn_table = NULL; | ||
| DBUG_RETURN(error); | ||
| } |
There was a problem hiding this comment.
If storage_open_indexes(name) fails and HA_OPEN_FOR_REPAIR is set, error is left with a non-zero value. Since we do not return early in this case, the function continues execution. If error is not reset to 0, storage_open may eventually return this non-zero error code, causing the open operation to fail despite HA_OPEN_FOR_REPAIR being specified to tolerate such failures.
We should reset error to 0 when we tolerate the failure under HA_OPEN_FOR_REPAIR.
error = storage_open_indexes(name);
if (error) {
if (!(open_options & HA_OPEN_FOR_REPAIR)) {
storage_close_columns();
grn_obj_unlink(ctx, grn_table);
grn_table = NULL;
DBUG_RETURN(error);
}
error = 0;
}
Many pathes exist for Mroonga to open tables without opening indexes. Altering from another storage engine and accessing the information schema statistics is one mechanism.
The fix is a backport from upstream.
Backport from mroonga/mroonga@0b35419#diff-c026dc94c7f79d426e7a68e1e69fdc0f7743296a37d6cd19c9e7117a763f242e
Fix a crash bug that may be caused after MySQL/MariaDB upgrade
GitHub: fix GH-423
MySQL/MariaDB may open Mroonga tables by "CHECK TABLES" when MySQL/MariaDB is upgraded. Mroonga didn't open indexes when a table is opened by "CHECK TABLES". If "REPAIR TABLE" is needed, a table is reopened. If "REPAIR TABLE" isn't needed, a table is NOT reopened. The table doesn't open indexes. "SELECT ... MATCH AGAINST" for the table causes a crash.
Reported by Vincent Pelletier. Thanks!!!
Backported to MariaDB by Daniel Black