Bug #69423 Double close() on the same file descriptor inside mysql_real_connect()
Submitted: 7 Jun 2013 11:16 Modified: 11 Feb 2015 16:10
Reporter: Yao Deng Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: C API (client library) Severity:S2 (Serious)
Version:5.6.19 OS:Linux
Assigned to: Igor Solodovnikov CPU Architecture:Any
Tags: connector/c

[7 Jun 2013 11:16] Yao Deng
Description:
Connector/C will invoke the system call close() twice on the file descriptor if the server isn't running.
This would be a serious problem if Connector/C is used in a multi-threaded program, since the second close() may close file descriptor created by other threads.

All of the following are based on the source code of the "Generic Linux (Architecture Independent), Compressed TAR Archive" version 6.1.0

The first close() call is at:
sql-common/client.c:3518: closesocket(sock);

backtrace of the second close() call is:
#0  inline_mysql_socket_close (mysql_socket=...)
    at /home/hoolala/workspace/search/third_party/mysql-connector-c-6.1.0-src/include/mysql/psi/mysql_socket.h:1099
#1  0x00007ffff6ffb807 in vio_shutdown (vio=0x7ffff0003190)
    at /home/hoolala/workspace/search/third_party/mysql-connector-c-6.1.0-src/vio/viosocket.c:417
#2  0x00007ffff6ffac51 in vio_delete (vio=0x7ffff0003190)
    at /home/hoolala/workspace/search/third_party/mysql-connector-c-6.1.0-src/vio/vio.c:339
#3  0x00007ffff6fc54a1 in end_server (mysql=0x7ffff00009e0)
    at /home/hoolala/workspace/search/third_party/mysql-connector-c-6.1.0-src/sql-common/client.c:962
#4  0x00007ffff6fcb8c3 in mysql_real_connect (mysql=0x7ffff00009e0, host=0x62bf18 "127.0.0.1", user=0x7ffff7071a2b "", 
    passwd=0x7ffff7071a2b "", db=0x0, port=9306, unix_socket=0x0, client_flag=131072)
    at /home/hoolala/workspace/search/third_party/mysql-connector-c-6.1.0-src/sql-common/client.c:3781

How to repeat:
Trying connecting any TCP port which isn't listened by any process through mysql_real_connect(). Don't set any fancy options.

Suggested fix:
Remove the first closesocket() ? I'm not sure ;p
[16 Jul 2014 12:30] Igor Solodovnikov
Posted by developer:
 
At first glance it looks like mysql_real_connect() should not call closesocket() when passing control to 'error' label and vio is already initialized with successfully created socket.

end_server(mysql) call which goes just after the 'error' label will free socket descriptor stored in vio structure.
[11 Feb 2015 16:10] Paul Dubois
Noted in 5.6.24, 5.7.5 changelogs.
Noted in Connector/C 6.1.6 changelog.

mysql_real_connect() could close a file descriptor twice if the
server was not running.
[27 Apr 2015 11:26] Laurynas Biveinis
commit 5fda8e03d76f6f1e2f18fe23ab1c49614dc5be44
Author: Igor Solodovnikov <igor.solodovnikov@oracle.com>
Date:   Wed Feb 11 01:44:06 2015 +0200

    Bug #19226740   DOUBLE CLOSE() ON THE SAME FILE DESCRIPTOR INSIDE MYSQL_REAL_CONNECT()
    
    In mysql_real_connect() when vio_socket_connect() fails we already have socket handle stored in net->vio. That means we should not explicitly call closesocket() at the end of the address list loop.
    
    If this is the last iteration of the loop then the code after the loop will pass control to the error label and end_server() call will close socket and free net->vio.
    
    If there will be another iteration of the loop then vio_reset() will be called with the new socket handle. Note that at the moment of vio_reset() call net->vio still owns the socket handle created on 
    
    Unfortunately vio_reset() did not close socket stored in Vio structure. This is why the patch makes following changes in vio_reset():
    - If new socket handle passed to vio_reset() is not equal to the socket handle stored in Vio then socket handle will be closed before storing new value. If handles are equal then old socket is not clo
    - If any error occurs then new vio_reset() implementation will not alter any Vio members thus preserving socket handle stored in Vio and not taking ownership over socket handle passed as parameter. Wi
    
    >  if (vio_reset(net->vio, VIO_TYPE_TCPIP, sock, NULL, flags))
    >  {
    >    set_mysql_error(mysql, CR_UNKNOWN_ERROR, unknown_sqlstate);
    >    closesocket(sock);    <--- close new handle
    >    freeaddrinfo(res_lst);
    >    if (client_bind_ai_lst)
    >      freeaddrinfo(client_bind_ai_lst);
    >    goto error;           <--- close handle stored in net->vio
    >  }
    
    If no error occurs then vio_reset() closes handle stored in net->vio and then stores new handle there. Otherwise explicit closesocket() call closes new handle and then control is passed to error label
    
    Reviewed-by: Rafal Somla <rafal.somla@oracle.com>
    Reviewed-by: Georgi Kodinov <georgi.kodinov@oracle.com>
    RB: 7973