← Back to team overview

oqgraph-dev team mailing list archive

Re: Serious use after free bug

 

From a very cursory look at federated it seems we dont have `handler::disconnect()` so I'll probably start there. Meantime in the medium term I think refactoring ha_oqgraph.cc to look more like ha_federated would be a worthwhile activity

Arjen Lentz <arjen@xxxxxxxxxxxxxxxx> wrote:

>Hi Andrew, Antony
>
>(specifically cc'ing Antony on the email as his email clients might blip 
>for that ;-)
>
>Does the graph share object potentially keep a reference to multiple 
>THDs, or only one? If only one, that'd be faulty as of course it can be 
>multiple.
>
>Then, I'm not sure THD is the thing that it should tally - using a 
>pointer as a reference can easily end up dangling loose in this way.
>
>Does a graph share object need to know exactly what its referenced by, 
>or is the issue simply that it needs to know when to self-destruct?
>If the latter, then a simple reference count would suffice.
>
>You may need some advice on how to ensure that when a connection closes, 
>things get cleaned up properly. We may have missed a hook of some sort. 
>You could also check this in another engine. Perhaps Federated, or 
>Spider, or CONNECT ?
>
>Regards,
>Arjen.
>
>
>On 17/06/14 22:08, Andrew McDonnell wrote:
>> Hi all,
>>
>> https://mariadb.atlassian.net/browse/MDEV-6282 is a use after free bug that
>> happens when one connection is opened, oqgraph queried, then the connection
>> closed, then a second connection opened and oqgraph queried again in the same
>> server session.
>>
>> Essentially, our handler in open() eventually creates a graph share object
>> that keeps a reference to the underlying TABLE object, this table object has
>> an in_use field that is a THD*.
>>
>> But when the connection goes away, mysqld free()'d that THD.  Except the
>> handler never processes close() at this point, and we never get a chance to
>> cause that table object to 'refresh' for want of a better word.
>>
>> Which means when the next query comes along, eventually it calls index_read()
>> on our handler which calls seek_to() in the graph code which calls back into
>> mysql using ha_index_read_map on the backing table, which then crashes because
>> it eventually calls increment_statistics which first increments free()'d
>> memory via &SSV and then accesses a bogus member pointer in the long dead THD.
>>
>> I dont yet know enough about the internals of mysqld to know how to handle this...
>>
>> cheers,
>> Andrew
>>
>
>
>-- 
>Arjen Lentz, Exec.Director @ Open Query (http://openquery.com.au)
>Australian peace of mind for your MySQL/MariaDB infrastructure.
>
>Follow us http://openquery.com.au/blog/ & http://twitter.com/openquery
>
>-- 
>Mailing list: https://launchpad.net/~oqgraph-dev
>Post to     : oqgraph-dev@xxxxxxxxxxxxxxxxxxx
>Unsubscribe : https://launchpad.net/~oqgraph-dev
>More help   : https://help.launchpad.net/ListHelp

Follow ups