[MDEV-31966] Server crash upon inserting into Mroonga table with compressed column Created: 2023-08-20  Updated: 2023-09-09  Resolved: 2023-09-09

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - Mroonga
Affects Version/s: 10.4, 10.5, 10.6, 10.10, 10.11, 11.0, 11.1
Fix Version/s: 10.4.32, 10.5.23, 10.6.16, 10.10.7, 10.11.6, 11.0.4, 11.1.3

Type: Bug Priority: Major
Reporter: Elena Stepanova Assignee: Sergei Golubchik
Resolution: Fixed Votes: 0
Labels: None


 Description    

INSTALL SONAME 'ha_mroonga';
 
CREATE TABLE t (a VARCHAR(128) COMPRESSED) ENGINE=Mroonga;
 
INSERT INTO t VALUES (REPEAT('a',128));
 
 
 
# Cleanup
 
DROP TABLE t;
 
UNINSTALL SONAME 'ha_mroonga';

10.4 900c4d69

 
#3  <signal handler called>
 
#4  0x0000558da26e2597 in Binary_string::alloc (this=0x8, arg_length=128) at /data/src/11.1/sql/sql_string.h:790
 
#5  0x0000558da329f81e in uncompress_zlib (to=0x0, from=0x61900009cecb "\200KL\034X", from_length=7, field_length=129) at /data/src/11.1/sql/field_comp.cc:110
 
#6  0x0000558da326c916 in Field_longstr::uncompress (this=0x61900009d010, val_buffer=0x0, val_ptr=0x7f89f2d79c50, from=0x61900009ceca "\211\200KL\034X", from_length=8) at /data/src/11.1/sql/field.cc:8468
 
#7  0x0000558da326cd9d in Field_varstring_compressed::val_str (this=0x61900009d010, val_buffer=0x0, val_ptr=0x7f89f2d79c50) at /data/src/11.1/sql/field.cc:8510
 
#8  0x00007f89f161ba9a in ha_mroonga::generic_store_bulk_variable_size_string (this=0x622000037938, field=0x61900009d010, buf=0x7f89f2d7a880) at /data/src/11.1/storage/mroonga/ha_mroonga.cpp:10343
 
#9  0x00007f89f1622965 in ha_mroonga::generic_store_bulk (this=0x622000037938, field=0x61900009d010, buf=0x7f89f2d7a880) at /data/src/11.1/storage/mroonga/ha_mroonga.cpp:10798
 
#10 0x00007f89f15dd43a in ha_mroonga::storage_write_row (this=0x622000037938, buf=0x61900009cec8 "\376\b\211\200KL\034X") at /data/src/11.1/storage/mroonga/ha_mroonga.cpp:6111
 
#11 0x00007f89f15e25b3 in ha_mroonga::write_row (this=0x622000037938, buf=0x61900009cec8 "\376\b\211\200KL\034X") at /data/src/11.1/storage/mroonga/ha_mroonga.cpp:6434
 
#12 0x0000558da32ff74a in handler::ha_write_row (this=0x622000037938, buf=0x61900009cec8 "\376\b\211\200KL\034X") at /data/src/11.1/sql/handler.cc:7804
 
#13 0x0000558da2907715 in write_record (thd=0x62b00007e218, table=0x61900009c998, info=0x7f89f2d7eb30, sink=0x0) at /data/src/11.1/sql/sql_insert.cc:2204
 
#14 0x0000558da28fea47 in mysql_insert (thd=0x62b00007e218, table_list=0x6290000e6358, fields=..., values_list=..., update_fields=..., update_values=..., duplic=DUP_ERROR, ignore=false, result=0x0) at /data/src/11.1/sql/sql_insert.cc:1154
 
#15 0x0000558da29c798f in mysql_execute_command (thd=0x62b00007e218, is_called_from_prepared_stmt=false) at /data/src/11.1/sql/sql_parse.cc:4449
 
#16 0x0000558da29de717 in mysql_parse (thd=0x62b00007e218, rawbuf=0x6290000e6238 "INSERT INTO t VALUES (REPEAT('a',128))", length=38, parser_state=0x7f89f2d7f9f0) at /data/src/11.1/sql/sql_parse.cc:7774
 
#17 0x0000558da29b6cba in dispatch_command (command=COM_QUERY, thd=0x62b00007e218, packet=0x629000258219 "INSERT INTO t VALUES (REPEAT('a',128))", packet_length=38, blocking=true) at /data/src/11.1/sql/sql_parse.cc:1892
 
#18 0x0000558da29b39f7 in do_command (thd=0x62b00007e218, blocking=true) at /data/src/11.1/sql/sql_parse.cc:1405
 
#19 0x0000558da2e74af0 in do_handle_one_connection (connect=0x608000002eb8, put_in_cache=true) at /data/src/11.1/sql/sql_connect.cc:1416
 
#20 0x0000558da2e744b1 in handle_one_connection (arg=0x608000002e38) at /data/src/11.1/sql/sql_connect.cc:1318
 
#21 0x0000558da3a707b2 in pfs_spawn_thread (arg=0x617000005b98) at /data/src/11.1/storage/perfschema/pfs.cc:2201
 
#22 0x00007f89fa8a7fd4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
 
#23 0x00007f89fa9285bc in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

Reproducible on all existing versions, debug- and non-debug builds with only slight differences in stack traces.

 Comments   
Comment by Kouhei Sutou [ 2023-09-04 ]

Does this work?

diff --git a/storage/mroonga/ha_mroonga.cpp b/storage/mroonga/ha_mroonga.cpp
 
index 85d6473ded3..5d4b1490397 100644
 
--- a/storage/mroonga/ha_mroonga.cpp
 
+++ b/storage/mroonga/ha_mroonga.cpp
 
@@ -10339,8 +10339,9 @@ int ha_mroonga::generic_store_bulk_variable_size_string(Field *field,
 
 {
 
   MRN_DBUG_ENTER_METHOD();
 
   int error = 0;
 
+  StringBuffer<MAX_FIELD_WIDTH> buffer(field->charset());
 
   String value;
 
-  field->val_str(NULL, &value);
 
+  field->val_str(&buffer, &value);
 
   grn_obj_reinit(ctx, buf, GRN_DB_SHORT_TEXT, 0);
 
   DBUG_PRINT("info", ("mroonga: length=%" MRN_FORMAT_STRING_LENGTH,
 
                       value.length()));

Comment by Sergei Golubchik [ 2023-09-04 ]

Thanks. That helped. Even better would be something like

--- a/storage/mroonga/ha_mroonga.cpp
 
+++ b/storage/mroonga/ha_mroonga.cpp
 
@@ -10366,13 +10366,13 @@ int ha_mroonga::generic_store_bulk_variable_size_string(Field *field,
 
 {
 
   MRN_DBUG_ENTER_METHOD();
 
   int error = 0;
 
-  String value;
 
-  field->val_str(NULL, &value);
 
+  StringBuffer<MAX_FIELD_WIDTH> buffer(field->charset());
 
+  String *value= field->val_str(&buffer, &buffer);
 
   grn_obj_reinit(ctx, buf, GRN_DB_SHORT_TEXT, 0);
 
   DBUG_PRINT("info", ("mroonga: length=%" MRN_FORMAT_STRING_LENGTH,
 
-                      value.length()));
 
-  DBUG_PRINT("info", ("mroonga: value=%s", value.c_ptr_safe()));
 
-  GRN_TEXT_SET(ctx, buf, value.ptr(), value.length());
 
+                      value->length()));
 
+  DBUG_PRINT("info", ("mroonga: value=%s", value->c_ptr_safe()));
 
+  GRN_TEXT_SET(ctx, buf, value->ptr(), value->length());
 
   DBUG_RETURN(error);
 
 }

because technically field->val_str() is not required to return any of its arguments.

Do you want me to apply this fix to Mroonga in MariaDB 10.4 or you do prefer to fix upstream and let us pull the fixed Mroonga before the next release (which will be around the end of October)?

Comment by Kouhei Sutou [ 2023-09-05 ]

Thanks.

Could you apply the fix to Mroonga in MariaDB?
I'll apply the same fix to upstream Mroonga but I can't send a pull request to merge upstream Mroonga to MariaDB by the next release.

Comment by Kouhei Sutou [ 2023-09-05 ]

We have a similar problem for TEXT COMPRESSED too. Could you also apply this?

diff --git a/storage/mroonga/ha_mroonga.cpp b/storage/mroonga/ha_mroonga.cpp
 
index 85d6473ded3..ffdad4037ff 100644
 
--- a/storage/mroonga/ha_mroonga.cpp
 
+++ b/storage/mroonga/ha_mroonga.cpp
 
@@ -10712,9 +10712,8 @@ int ha_mroonga::generic_store_bulk_blob(Field *field, grn_obj *buf)
 
 {
 
   MRN_DBUG_ENTER_METHOD();
 
   int error = 0;
 
-  String buffer;
 
-  Field_blob *blob = (Field_blob *)field;
 
-  String *value = blob->val_str(0, &buffer);
 
+  StringBuffer<MAX_FIELD_WIDTH> buffer(field->charset());
 
+  auto value = field->val_str(&buffer, &buffer);
 
   grn_obj_reinit(ctx, buf, GRN_DB_TEXT, 0);
 
   GRN_TEXT_SET(ctx, buf, value->ptr(), value->length());
 
   DBUG_RETURN(error);

Generated at Thu Feb 08 10:27:48 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.