[MDEV-25994] Crash with union of my_decimal type in ORDER BY clause Created: 2021-06-23  Updated: 2023-12-12  Resolved: 2022-04-22

Status: Closed
Project: MariaDB Server
Component/s: Optimizer
Affects Version/s: 5.5, 10.0, 10.1, 10.2, 10.3, 10.4, 10.5, 10.6
Fix Version/s: 10.2.44, 10.3.35, 10.4.25, 10.5.16, 10.6.8, 10.7.4, 10.8.3, 10.9.1

Type: Bug Priority: Blocker
Reporter: Vicențiu Ciorbaru Assignee: Sergei Petrunia
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Duplicate
is duplicated by MDEV-26280 MariaDB server crash at my_decimal::... Closed
is duplicated by MDEV-26404 A SEGV insql/filesort.cc Closed
is duplicated by MDEV-27080 Malicious data type overflow in joint... Closed
Relates
relates to MDEV-29019 Assertion `(length % 4) == 0' failed ... Closed
relates to MDEV-32324 Server crashes inside filesort at my_... Closed

 Description   

Steps to reproduce:

CREATE TABLE v0 ( v1 INTEGER ) ;
 
INSERT INTO v0 ( v1 ) VALUES ( 8 ) ;
 
UPDATE v0 SET v1 = 1 ORDER BY ( SELECT 1.1 UNION SELECT -1 );


sigaction.c:0(__restore_rt)[0x7f5b729fc870]
 
sql/my_decimal.h:132(my_decimal::operator=(my_decimal const&))[0x55e7157e1e30]
 
sql/my_decimal.h:354(my_decimal2decimal(my_decimal const*, my_decimal*))[0x55e7157e2011]
 
sql/my_decimal.cc:207(my_decimal::to_binary(unsigned char*, int, unsigned short, unsigned int) const)[0x55e715a84a04]
 
sql/filesort.cc:1321(Type_handler_decimal_result::make_sort_key_part(unsigned char*, Item*, SORT_FIELD_ATTR const*, Sort_param*) const)[0x55e7158e8810]
 
sql/filesort.cc:3030(make_sortkey(Sort_param*, unsigned char*))[0x55e7158ecfe6]
 
sql/filesort.cc:1352(make_sortkey(Sort_param*, unsigned char*, unsigned char*, bool))[0x55e7158e8933]
 
sql/filesort.cc:969(find_all_keys(THD*, Sort_param*, SQL_SELECT*, SORT_INFO*, st_io_cache*, st_io_cache*, Bounded_queue<unsigned char, unsigned char>*, unsigned long long*))[0x55e7158e7592]
 
sql/filesort.cc:357(filesort(THD*, TABLE*, Filesort*, Filesort_tracker*, JOIN*, unsigned long long))[0x55e7158e53fb]
 
sql/sql_update.cc:796(mysql_update(THD*, TABLE_LIST*, List<Item>&, List<Item>&, Item*, unsigned int, st_order*, unsigned long long, bool, unsigned long long*, unsigned long long*))[0x55e7156b1588]
 
sql/sql_parse.cc:4399(mysql_execute_command(THD*))[0x55e71557ebb4]
 
sql/sql_parse.cc:8016(mysql_parse(THD*, char*, unsigned int, Parser_state*))[0x55e71558ad79]
 
sql/sql_parse.cc:1899(dispatch_command(enum_server_command, THD*, char*, unsigned int, bool))[0x55e7155773a8]
 
sql/sql_parse.cc:1406(do_command(THD*, bool))[0x55e715575d6b]
 
sql/sql_connect.cc:1410(do_handle_one_connection(CONNECT*, bool))[0x55e71572ca46]
 
sql/sql_connect.cc:1314(handle_one_connection)[0x55e71572c7b1]
 
perfschema/pfs.cc:2203(pfs_spawn_thread)[0x55e715c4180a]
 
pthread_create.c:0(start_thread)[0x7f5b729f2259]
 
:0(__GI___clone)[0x7f5b7259d5e3]



 Comments   
Comment by Alice Sherepa [ 2021-06-23 ]

Repeatable on 5.5-10.6
on non-debug build:

210623  8:59:06 [ERROR] mysqld got signal 11 ;
 
Server version: 10.5.10-MariaDB
 
 
 
mysys/stacktrace.c:213(my_print_stacktrace)[0x5562338ee4d7]
 
??:0(__restore_rt)[0x7f1697ec1730]
 
sql/my_decimal.h:128(my_decimal::to_binary(unsigned char*, int, int, unsigned int) const)[0x556233a125c4]
 
sql/filesort.cc:1309(Type_handler_decimal_result::make_sort_key_part(unsigned char*, Item*, SORT_FIELD_ATTR const*, Sort_param*) const)[0x5562338e97fb]
 
sql/filesort.cc:3016(make_sortkey)[0x5562338eabc7]
 
sql/filesort.cc:958(find_all_keys)[0x5562338ed03d]
 
sql/sql_update.cc:794(mysql_update(THD*, TABLE_LIST*, List<Item>&, List<Item>&, Item*, unsigned int, st_order*, unsigned long long, bool, unsigned long long*, unsigned long long*))[0x5562337991e1]
 
sql/sql_parse.cc:4481(mysql_execute_command(THD*))[0x5562336e89c0]
 
sql/sql_parse.cc:8099(mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool))[0x5562336eaefc]
 
sql/sql_parse.cc:1951(dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool))[0x5562336ed93a]
 
sql/sql_parse.cc:1375(do_command(THD*))[0x5562336eec6f]
 
sql/sql_connect.cc:1410(do_handle_one_connection(CONNECT*, bool))[0x5562337dbb12]
 
sql/sql_connect.cc:1318(handle_one_connection)[0x5562337dbdd4]
 
perfschema/pfs.cc:2204(pfs_spawn_thread)[0x556233b68c4d]
 
nptl/pthread_create.c:487(start_thread)[0x7f1697eb6fa3]
 
x86_64/clone.S:97(clone)[0x7f1697ad94cf]
 
 
 
Query (0x7f1648011e10): UPDATE v0 SET v1 = 1 ORDER BY ( SELECT 1.1 UNION SELECT -1 )

Comment by Oleg Smirnov [ 2022-04-16 ]

1. The cause of the crash is NULL value of dec_val pointer retrieved in this function:

10.5 filesort.cc lines 1302-1318

 
void
 
Type_handler_decimal_result::make_sort_key_part(uchar *to, Item *item,
 
                                            const SORT_FIELD_ATTR *sort_field,
 
                                            Sort_param *param) const
 
{
 
  my_decimal dec_buf, *dec_val= item->val_decimal_result(&dec_buf);
 
  if (item->maybe_null())
 
  {
 
    if (item->null_value)
 
    {
 
      memset(to, 0, sort_field->length + 1);
 
      return;
 
    }
 
    *to++= 1;
 
  }
 
  dec_val->to_binary(to, item->max_length - (item->decimals ? 1 : 0),
 
                     item->decimals);
 
}

dec_val isn't analyzed for NULL so dec_val->to_binary() is called and that leads to the crash.

And the cause of NULL value is the failure to execute a single row subselect due to more than one row in the result:

10.5 sql_class.cc lines 3673-3689

 
int select_singlerow_subselect::send_data(List<Item> &items)
 
{
 
  DBUG_ENTER("select_singlerow_subselect::send_data");
 
  Item_singlerow_subselect *it= (Item_singlerow_subselect *)item;
 
  if (it->assigned())
 
  {
 
    my_message(ER_SUBQUERY_NO_1_ROW, ER_THD(thd, ER_SUBQUERY_NO_1_ROW),
 
               MYF(current_thd->lex->ignore ? ME_JUST_WARNING : 0));
 
    DBUG_RETURN(1);
 
  }

Looking for the ways to process this situation correctly.

2. Does it make any sense to evaluate ORDER BY for an UPDATE that does not have a LIMIT clause? Maybe we can eliminate such ORDER BY at an early stage?

Comment by Oleg Smirnov [ 2022-04-17 ]

3. I pushed the fix to bb-10.2-MDEV-25994 but this code was changed in 10.5, so merging may be non-trivial.

Comment by Oleg Smirnov [ 2022-04-18 ]

Buildbot tests have succeded (https://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.2-MDEV-25994), so I'm passing this to review.

Comment by Sergei Petrunia [ 2022-04-19 ]

I see one more problem in Type_handler_decimal_result::make_sort_key:

(gdb) p dbug_print_item(item)
 
  $31 = 0x5555570e2b80 <dbug_item_print_buf> "(subquery#2)"
 
(gdb) p item->maybe_null
 
  $32 = false
 
(gdb) p item->null_value
 
  $33 = true
 
(gdb) p item
 
  $34 = (Item_singlerow_subselect *) 0x7fff6c0155a0

That is, Item_singlerow_subselect has maybe_null=FALSE, but after val_decimal() it has null_value=TRUE...

Comment by Sergei Petrunia [ 2022-04-20 ]

Trying to debug a different query:

select 1.0 +( SELECT 1.1 UNION SELECT -1 ) from v0;
 
ERROR 1242 (21000): Subquery returns more than 1 row

Internally, the same happens:
Item_singlerow_subselect has maybe_null=false, null_value=true.
These properties are passed over to Item_func_plus.

Then, there is a check for thd->is_error(), after which the query execution is aborted.

Comment by Sergei Petrunia [ 2022-04-20 ]

A proposed alternative patch:

diff --git a/sql/filesort.cc b/sql/filesort.cc
 
index 8b019caf8f5..6b9e8af6323 100644
 
--- a/sql/filesort.cc
 
+++ b/sql/filesort.cc
 
@@ -1118,7 +1118,7 @@ Type_handler::make_sort_key_longlong(uchar *to,
 
                                      longlong value) const
 
 
 {
 
-  if (maybe_null)
 
+ // if (maybe_null)
 
   {
 
     if (null_value)
 
     {
 
@@ -1147,7 +1147,7 @@ Type_handler_decimal_result::make_sort_key(uchar *to, Item *item,
 
                                            Sort_param *param) const
 
 {
 
   my_decimal dec_buf, *dec_val= item->val_decimal_result(&dec_buf);
 
-  if (item->maybe_null)
 
+  //if (item->maybe_null)
 
   {
 
     if (item->null_value)
 
     {
 
@@ -1168,7 +1168,7 @@ Type_handler_real_result::make_sort_key(uchar *to, Item *item,
 
                                         Sort_param *param) const
 
 {
 
   double value= item->val_result();
 
-  if (item->maybe_null)
 
+  //if (item->maybe_null)
 
   {
 
     if (item->null_value)
 
     {

The idea is to follow the approach in other places and handle the case when item->null_value==true even for items that have maybe_null==false. (I'm not sure if I've fixed all places in make_sort_key code)

Comment by Sergei Petrunia [ 2022-04-20 ]

Oleg's patch:

  • makes Type_handler_XX::make_sort_key() return bool.
  • Makes the implementations return true when the underlying item has maybe_null=false && null_value=true (TODO: is this the only case where they return true?)

if make_sort_key() returns true only on error... well, one can check for error condition by looking at thd->is_error() ...

Comment by Sergei Petrunia [ 2022-04-20 ]

Take-aways from the call: it seems the best way forward is to make Item_singlerow_subselect NULLable when appropriate

Comment by Sergei Petrunia [ 2022-04-21 ]

https://github.com/MariaDB/server/commit/baec17bc033cb9be02428e2089046df9f530754a

Comment by Sergei Petrunia [ 2022-04-21 ]

sanja, please review the second patch variant: https://github.com/MariaDB/server/commit/baec17bc033cb9be02428e2089046df9f530754a

Comment by Oleksandr Byelkin [ 2022-04-22 ]

OK to push.

Comment by Oleg Smirnov [ 2022-04-25 ]

And what do you think about my comment #2:
"2. Does it make any sense to evaluate ORDER BY for an UPDATE that does not have a LIMIT clause? Maybe we can eliminate such ORDER BY at an early stage?"

Generated at Thu Feb 08 09:41:57 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.