Skip to content

MDEV-39556 SIGSEGV in ha_mroonga::storage_set_keys_in_use on SELECT#5213

Open
grooverdan wants to merge 1 commit into
MariaDB:10.6from
grooverdan:MDEV-39556
Open

MDEV-39556 SIGSEGV in ha_mroonga::storage_set_keys_in_use on SELECT#5213
grooverdan wants to merge 1 commit into
MariaDB:10.6from
grooverdan:MDEV-39556

Conversation

@grooverdan

Copy link
Copy Markdown
Member

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

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
@grooverdan grooverdan requested a review from sanja-byelkin June 10, 2026 08:00
@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label Jun 10, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +4290 to +4298
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

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;

Comment on lines +4659 to +4665
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);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

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;
  }

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

Labels

MariaDB Foundation Pull requests created by MariaDB Foundation

Development

Successfully merging this pull request may close these issues.

3 participants