Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 35 additions & 39 deletions storage/mroonga/ha_mroonga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4287,18 +4287,15 @@ int ha_mroonga::wrapper_open(const char *name, int mode, uint open_options)
if (error)
DBUG_RETURN(error);

if (!(open_options & HA_OPEN_FOR_REPAIR)) {
error = open_table(name);
if (error)
DBUG_RETURN(error);
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;
Comment on lines +4290 to +4298

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;


mrn_init_alloc_root(&mem_root, 1024, 0, MYF(0));
wrap_key_info = mrn_create_key_info_for_table(share, table, &error);
Expand Down Expand Up @@ -4386,6 +4383,7 @@ int ha_mroonga::wrapper_open(const char *name, int mode, uint open_options)
}
}

error_exit:
if (error)
{
grn_obj_unlink(ctx, grn_table);
Expand Down Expand Up @@ -4658,37 +4656,35 @@ int ha_mroonga::storage_open(const char *name, int mode, uint open_options)
DBUG_RETURN(error);
}

if (!(open_options & HA_OPEN_FOR_REPAIR)) {
error = storage_open_indexes(name);
if (error) {
storage_close_columns();
grn_obj_unlink(ctx, grn_table);
grn_table = NULL;
DBUG_RETURN(error);
}
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);
}
Comment on lines +4659 to +4665

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


storage_set_keys_in_use();
storage_set_keys_in_use();

{
mrn::Lock lock(&mrn_operations_mutex);
mrn::PathMapper mapper(name);
const char *table_name = mapper.table_name();
size_t table_name_size = strlen(table_name);
if (db->is_broken_table(table_name, table_name_size)) {
GRN_LOG(ctx, GRN_LOG_NOTICE,
"Auto repair is started: <%s>",
name);
error = operations_->repair(table_name, table_name_size);
if (!(open_options & HA_OPEN_FOR_REPAIR)) {
mrn::Lock lock(&mrn_operations_mutex);
mrn::PathMapper mapper(name);
const char *table_name = mapper.table_name();
size_t table_name_size = strlen(table_name);
if (db->is_broken_table(table_name, table_name_size)) {
GRN_LOG(ctx, GRN_LOG_NOTICE,
"Auto repair is started: <%s>",
name);
error = operations_->repair(table_name, table_name_size);
if (!error)
db->mark_table_repaired(table_name, table_name_size);
if (!share->disable_keys) {
if (!error)
db->mark_table_repaired(table_name, table_name_size);
if (!share->disable_keys) {
if (!error)
error = storage_reindex();
}
GRN_LOG(ctx, GRN_LOG_NOTICE,
"Auto repair is done: <%s>: %s",
name, error == 0 ? "success" : "failure");
error = storage_reindex();
}
GRN_LOG(ctx, GRN_LOG_NOTICE,
"Auto repair is done: <%s>: %s",
name, error == 0 ? "success" : "failure");
}
}

Expand Down Expand Up @@ -5233,7 +5229,7 @@ void ha_mroonga::storage_set_keys_in_use()
if (i == table_share->primary_key) {
continue;
}
if (!grn_index_tables[i]) {
if (grn_index_tables && !grn_index_tables[i]) {
/* disabled */
table_share->keys_in_use.clear_bit(i);
DBUG_PRINT("info", ("mroonga: key %u disabled", i));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ INSERT INTO diaries VALUES ('Hello');
FLUSH TABLES;
CHECK TABLE diaries;
Table Op Msg_type Msg_text
check_test.diaries check error Corrupt
check_test.diaries check status OK
REPAIR TABLE diaries;
Table Op Msg_type Msg_text
check_test.diaries repair status OK
SELECT *
FROM diaries
WHERE MATCH(title) AGAINST('+Hello' IN BOOLEAN MODE);
title
Hello
DROP TABLE diaries;
DROP DATABASE check_test;
USE test;
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#
# MDEV-39556 SIGSEGV in ha_mroonga::storage_set_keys_in_use on SELECT
#
CREATE TABLE t (c INT KEY,c2 GEOMETRY NOT NULL,SPATIAL INDEX idx_sp (c2)) ENGINE=InnoDB;
ALTER TABLE t ENGINE=Mroonga;
CHECK TABLE t;
Table Op Msg_type Msg_text
test.t check status OK
SELECT COUNT(*) > 0 AS r FROM information_schema.STATISTICS;
r
1
DROP TABLE t;
# End of 10.6 tests
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ SELECT * FROM diaries WHERE MATCH(body) AGAINST("+starting" IN BOOLEAN MODE);
ERROR HY000: system call error: No such file or directory: failed to open path: <repair_test.mrn.000010E.c>
REPAIR TABLE diaries;
Table Op Msg_type Msg_text
repair_test.diaries repair Error system call error: No such file or directory: failed to open path: <repair_test.mrn.000010E.c>
repair_test.diaries repair status OK
SELECT * FROM diaries;
id title body
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ CREATE TABLE diaries (

INSERT INTO diaries VALUES ('Hello');

--remove_file $MYSQLD_DATADIR/check_test.mrn.000010C

FLUSH TABLES;

CHECK TABLE diaries;

REPAIR TABLE diaries;

SELECT *
FROM diaries
WHERE MATCH(title) AGAINST('+Hello' IN BOOLEAN MODE);

DROP TABLE diaries;

DROP DATABASE check_test;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
--source include/have_innodb.inc

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

CREATE TABLE t (c INT KEY,c2 GEOMETRY NOT NULL,SPATIAL INDEX idx_sp (c2)) ENGINE=InnoDB;

ALTER TABLE t ENGINE=Mroonga;
CHECK TABLE t;
SELECT COUNT(*) > 0 AS r FROM information_schema.STATISTICS;

DROP TABLE t;

--echo # End of 10.6 tests
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ FLUSH TABLES;
--error ER_CANT_OPEN_FILE
SELECT * FROM diaries WHERE MATCH(body) AGAINST("+starting" IN BOOLEAN MODE);

--replace_result "[io][open] " ""
REPAIR TABLE diaries;

SELECT * FROM diaries;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ SELECT * FROM diaries WHERE MATCH(body) AGAINST("starting");
ERROR HY000: mroonga: failed to open table: <diaries>
REPAIR TABLE diaries;
Table Op Msg_type Msg_text
repair_test.diaries repair Error mroonga: failed to open table: <diaries>
repair_test.diaries repair status OK
SELECT * FROM diaries;
id title body
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ SELECT * FROM diaries WHERE MATCH(body) AGAINST("starting");
ERROR HY000: system call error: No such file or directory: failed to open path: <repair_test.mrn.000010A>
REPAIR TABLE diaries;
Table Op Msg_type Msg_text
repair_test.diaries repair Error system call error: No such file or directory: failed to open path: <repair_test.mrn.000010A>
repair_test.diaries repair status OK
SELECT * FROM diaries;
id title body
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ FLUSH TABLES;
--error ER_CANT_OPEN_FILE
SELECT * FROM diaries WHERE MATCH(body) AGAINST("starting");

--replace_result "[io][open] " ""
REPAIR TABLE diaries;

SELECT * FROM diaries;
Expand Down