MySQL Bugs: #42675: Dangling pointer leads to a client crash (mysys/my

Bug #42675	Dangling pointer leads to a client crash (mysys/my_error.c patch enclosed)
Submitted:	8 Feb 2:37	Modified:	13 Apr 21:26
Reporter:	Alex Goncharov
Status:	Closed
Category:	C API	Severity:	S2 (Serious)
Version:	All (5.1.30, 5.1.31, 6.0)	OS:	Any
Assigned to:	Bugs System	Target Version:	5.1+
Tags:	Contribution, client odbc disconnect cleanup environment
Triage:	Triaged: D2 (Serious)

Description:
This patch explains and fixes the problem:

============================================================
--- mysys/my_error.c~   2008-11-14 11:36:58.000000000 -0500
+++ mysys/my_error.c    2009-02-04 14:34:05.000000000 -0500
@@ -257,4 +257,5 @@ void my_error_unregister_all(void)
     my_free((uchar*) list, MYF(0));
   }
   my_errmsgs_list= &my_errmsgs_globerrs;
+  my_errmsgs_list->meh_next = NULL;
 }
============================================================

If a client process makes more than one connection to MySQL, then on
closing the first connection `my_errmsgs_list' gets repointed to
`my_errmsgs_globerrs', which is correct.  But `my_errmsgs_globerrs' is
not returned to the state which it was in the beginning of the program
(my_errmsgs_globerrs = {NULL, ... }) -- its `meh_next' member points
to the memory area just cleaned out.

When the second connection is closed, this happens (pre-patch):

------------------------------------------------------------
Exiting on signal SIGSEGV (11)

#8  my_error_unregister_all () at my_error.c:256
#9  0x0083f6fb in my_end (infoflag=<value optimized out>) at my_init.c:164
#10 0x00386fc1 in myodbc_end () at dll.c:117
#11 0x0038a736 in my_SQLFreeEnv (henv=0x9943558) at handle.c:114
#12 0x0038a7b8 in SQLFreeHandle (HandleType=1, Handle=0xbfcd5724) at handle.c:632
#13 0x002fc339 in release_env (connection=0x9944990)
------------------------------------------------------------

Applying the enclosed patch resolves the problem.

How to repeat:
Write an ODBC program doing opening two connections, then closing them.

Suggested fix:
--- mysys/my_error.c~   2008-11-14 11:36:58.000000000 -0500
+++ mysys/my_error.c    2009-02-04 14:34:05.000000000 -0500
@@ -257,4 +257,5 @@ void my_error_unregister_all(void)
     my_free((uchar*) list, MYF(0));
   }
   my_errmsgs_list= &my_errmsgs_globerrs;
+  my_errmsgs_list->meh_next = NULL;
 }

Thank you for the bug report. Could you please provide your sample test code?.
For example are you using the same MYSQL structure for both connections?. Thanks in
advance.

> Could you please provide your sample test code?.

Submitting the code is very problematic and will not help much...

> For example are you using the same MYSQL structure for both connections?. 

Not necesserily: the structure in question is the same for the life time of 
the client process (ODBC or other).

If that client process makes two connections, two acts of cleaning up the
`my_errmsgs_globerrs' structure will happen: the first one will see the structure in good
state and will succeed, but leave the structure in a bad state,

The second connection, no matter how originated, when it's cleaned up, will run
into the `my_errmsgs_globerrs' in an invalid state (i.e. if my patch is not applied.)

Tested by various runs, going back and forth between bad and good behaviours :-)

Thank you for the feedback and contribution patch.

You are welcome!

That's why I started my submission with the (tiny) patch -- I hope the patch 
illustrates the problem better than human words :-)

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/69484

2841 Chad MILLER	2009-03-17
      Bug#42675: Dangling pointer leads to a client crash (mysys/my_error.c \
      	patch enclosed)
        
      One call to my_error_unregister_all() would free pointers, but leave one
      pointer to just-freed memory still assigned.  That's the bug.  Subsequent
      calls of this function would try to follow pointers into deallocated, 
      garbage memory and almost certainly SEGV.
      
      Now, after freeing a linked list, unset the initial pointer.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/69485

2841 Chad MILLER	2009-03-17
      Bug#42675: Dangling pointer leads to a client crash (mysys/my_error.c \
      	patch enclosed)
        
      One call to my_error_unregister_all() would free pointers, but leave one
      pointer to just-freed memory still assigned.  That's the bug.  Subsequent
      calls of this function would try to follow pointers into deallocated, 
      garbage memory and almost certainly SEGV.
      
      Now, after freeing a linked list, unset the initial pointer.

Queued to 5.1- and 6.0-bugteam trees.

Pushed into 5.1.34 (revid:joro@sun.com-20090327143448-wuuuycetc562ty6o) (version source
revid:leonard@mysql.com-20090316090622-sr8lylqvsl1jrcnv) (merge vers: 5.1.34) (pib:6)

Noted in 5.1.34 changelog.

A dangling pointer in mysys/my_error.c could lead to client crashes.

Setting report to NDI pending push into 6.0.x.

Pushed into 6.0.11-alpha (revid:alik@sun.com-20090413084402-snnrocwzktcl88ny) (version
source revid:chad@mysql.com-20090318224105-ieuytuh37hz80x16) (merge vers: 6.0.11-alpha)
(pib:6)

Noted in 6.0.11 changelog.

Pushed into 5.1.34-ndb-6.2.18 (revid:jonas@mysql.com-20090508185236-p9b3as7qyauybefl)
(version source revid:jonas@mysql.com-20090508185236-p9b3as7qyauybefl) (merge vers:
5.1.34-ndb-6.2.18) (pib:6)

Pushed into 5.1.34-ndb-6.3.25 (revid:jonas@mysql.com-20090509063138-1u3q3v09wnn2txyt)
(version source revid:jonas@mysql.com-20090509063138-1u3q3v09wnn2txyt) (merge vers:
5.1.34-ndb-6.3.25) (pib:6)

Pushed into 5.1.34-ndb-7.0.6 (revid:jonas@mysql.com-20090509154927-im9a7g846c6u1hzc)
(version source revid:jonas@mysql.com-20090509154927-im9a7g846c6u1hzc) (merge vers:
5.1.34-ndb-7.0.6) (pib:6)