[MDEV-18970] uninited var can be read in gtid_delete_pending() Created: 2019-03-19  Updated: 2019-05-16  Resolved: 2019-05-16

Status: Closed
Project: MariaDB Server
Component/s: Replication
Affects Version/s: 10.4.1
Fix Version/s: 10.4.5

Type: Bug Priority: Major
Reporter: Andrei Elkin Assignee: Sujatha Sivakumar (Inactive)
Resolution: Fixed Votes: 0
Labels: None


 Description   

marko reports 

gcc 8 -O2 seems to indicate a real error for this code:
 
    table_opened= true;
 
    table= tlist.table;
 
 
 
    if ((err= gtid_check_rpl_slave_state_table(table)))
 
      goto end;
 
 
 
    direct_pos= table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION;

the warning:
/mariadb/10.4/sql/rpl_gtid.cc:980:7: warning: 'direct_pos' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (!direct_pos)
^~
that code in question is after end:, and inside if (table_opened) (which has been initialized above, but direct_pos is not if we do the above goto end)

The following patch should suffice:

diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc
 
index 17f474c2acf..ab74ab74687 100644
 
--- a/sql/rpl_gtid.cc
 
+++ b/sql/rpl_gtid.cc
 
@@ -904,9 +904,6 @@ rpl_slave_state::gtid_delete_pending(THD *thd,
 
     table_opened= true;
 
     table= tlist.table;
 
 
-    if ((err= gtid_check_rpl_slave_state_table(table)))
 
-      goto end;
 
-
 
     direct_pos= table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION;
 
     bitmap_set_all(table->write_set);
 
     table->rpl_write_set= table->write_set;
 
@@ -921,6 +918,9 @@ rpl_slave_state::gtid_delete_pending(THD *thd,
 
       goto end;
 
     }
 
 
+    if ((err= gtid_check_rpl_slave_state_table(table)))
 
+      goto end;
 
+
 
     cur = *list_ptr;
 
     cur_ptr_ptr = list_ptr;
 
     do

as there's no urgency to call gtid_check_rpl_slave_state_table() earlier than the var-in-question gets assigned.

 Comments   
Comment by Sujatha Sivakumar (Inactive) [ 2019-03-29 ]

Hello Andrei,

Please review the fix for MDEV-18970. The patch is available at

http://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.4-sujatha

To reproduce I followed these instructions:

sujatha@sujatha:~/bug_repo/MDEV-18970-10.4/bld$ echo $CC
/usr/bin/gcc-8
sujatha@sujatha:~/bug_repo/MDEV-18970-10.4/bld$ echo $CXX
/usr/bin/g++-8
sujatha@sujatha:~/bug_repo/MDEV-18970-10.4/bld$ echo $CFLAGS
-O2
sujatha@sujatha:~/bug_repo/MDEV-18970-10.4/bld$ echo $CXXFLAGS
-O2
cmake command:
===============
MDEV-18970-10.4/bld$ cmake -DCONC_WITH_

{UNITTEST,SSL}

=OFF -DWITH_EMBEDDED_SERVER=OFF -DWITH_UNIT_TESTS=OFF -DCMAKE_BUILD_TYPE=Debug -DPLUGIN_TOKUDB=NO -DPLUGIN_MROONGA=NO -DPLUGIN_OQGRAPH=NO -DPLUGIN_ROCKSDB=NO -DPLUGIN_CONNECT=NO -DWITH_SSL=bundled ..

Please let me know if you need additional details.

Thanks.

Comment by Sujatha Sivakumar (Inactive) [ 2019-03-29 ]

Fixed the patch specific issue in my earlier version Andrei. Tested the changes locally.
I have submitted to bb-10.4-sujatha. But it seems to be taking a lot of time.
Please have a look at the new changes.

Comment by Andrei Elkin [ 2019-05-15 ]

Sujatha, salute.

The patch looks good. I would suggest to assert instead of comment in the last hunk.
Something like assert(direct_pos || index_inited || thd->is_error()).

Thanks for this!

Andrei

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