Bug #88428 mysql_real_query hangs with EINTR errno (using YASSL)
Submitted: 9 Nov 2017 22:29 Modified: 31 May 13:40
Reporter: Steve Brisson Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: Security: Encryption Severity:S2 (Serious)
Version:5.7.20 OS:Ubuntu
Assigned to: CPU Architecture:Any

[9 Nov 2017 22:29] Steve Brisson
Description:
OS: Ubuntu 16.04.3 LTS server
Relevant libs: libmysqlclient.so.20.3.7

I have a client program running mysql queries against a remote db and also forking to exec programs on a separate process. One of my test instances is repeatedly experiencing a hang in mysql_real_query (process consuming 100% cpu) and does not recover.

How to repeat:
I built a debug version of libmysqlclient.so.20.3.7 and found that the program is spinning in the while loop in the function net_read_raw_loop [sql/net_serv.cc]. In this state, SSL_read is failing with a SSL_get_error of 114 (YasslError receive_error) and because errno is EINTR (not sure what system call set this originally), the net_should_retry [sql/net_serv.cc] function will always return true.

This results in an infinite loop that will not ever recover.

Suggested fix:
Temporarily I have hack/patched the issue by treating yassl errors the same as SSL_ERROR_SSL in the ssl_set_sys_error [vio/viossl.c] function and also clearing errno after an EINTR is detected in vio_should_retry [vio/viosocket.c].

Looking for feedback on if there is an issue with yassl error handling in libmysqlclient or I have simply hacked around some other problem.
[10 Nov 2017 15:51] Miguel Solorzano
Thank you for the bug report. Please provide test case (C code file) attaching it using the Files tab and the command line you used to compile. Thanks in advance.
[10 Nov 2017 16:50] Steve Brisson
I don't have a good test case to reproduce the problem. The mysql client code I am using is part of a larger software package. The issue also seems very timing dependent so the size of the database is a factor as well. I was hoping code inspection would be enough to identify an issue or clear any misunderstandings I might have about how the code is supposed to work.

Again, I think my problem starts with SSL_get_error returning a 114 and this not being handled in viossl. That, combined with errno being set to EINTR creates the infinite loop in net_read_raw_loop.
[14 Nov 2017 12:04] Chiranjeevi Battula
Hello  Steve,

Thank you for the feedback.
Could you please provide repeatable test case, server logs (exact steps/sample code,  return code from server etc. - please make it as private if you prefer) to confirm this issue at our end?

Thanks,
Chiranjeevi.
[20 Nov 2017 17:09] Steve Brisson
I don't understand what you are asking for here. I don't have test code or server logs as I don't think they are relevant.

This is an issue I am having in libmysqlclient and I have given you the lines of code and errno state which is causing a hang. I believe that this could happen on any mysql client and would not be dependent on implementation.
[21 Nov 2017 14:17] Sinisa Milivojevic
Hi!

We have taken a deep lock at the problem you are describing and this is indeed a small bug in how we are using SSL library. This is a small optimisation that has number of glitches, like the one you described.

Verified as reported.
[28 Dec 2017 13:49] Sinisa Milivojevic
Hi!

We would like to inform you that we are working on this bug, but in order to proceed, we do need you to confirm the scenario in which this bug is occurring.

So, please, answer the following two questions:

* Can you confirm that the hang in mysql_real_query and that it occurs because of constant EINTR error from the operating system ???

* Also, please, let us know whether you are using YaSSL and you are not using at all OpenSSL ??? Did you try OpenSSL ????

Thank you very much in advance.
[28 Dec 2017 19:28] Steve Brisson
* Can you confirm that the hang in mysql_real_query and that it occurs because of constant EINTR error from the operating system ???

Yes, I only experienced the hang when errno was EINTR.

* Also, please, let us know whether you are using YaSSL and you are not using at all OpenSSL ??? Did you try OpenSSL ????

I am using YaSSL. I did not try OpenSSL.
[29 Dec 2017 12:31] Sinisa Milivojevic
Thank you for your feedback.
[29 Jan 2018 14:12] Sinisa Milivojevic
A patch to get a verbose output from YaSSL EINTR problems

Attachment: yassl_verbose_29-01.patch (application/octet-stream, text), 5.95 KiB.

[29 Jan 2018 14:14] Sinisa Milivojevic
Hi Mr. Brisson,

We have problem in repeating the situation that you describe. Hence, therefore, we have left in the "Files" tab a code patch. Please, apply a code patch, regenerate the problematic situation of YaSSL and when you see verbose diagnostic in your error log, please upload it to the "Files" tab.

Let us know if you have any question.
[20 Feb 2018 13:33] Sinisa Milivojevic
Hi!

Can you please try the patch that we have attached here ????

It is very important for us.
[21 Feb 2018 17:13] Steve Brisson
I'm no longer in a position where I can make test changes to the server so I am unable to try out your patch.

My own solution was to handle yassl errors in the ssl_set_sys_error function in viossl.c. In the ssl_error switch statement you simply need to set the variable 'error' appropriately on a yassl error (similar to how case SSL_ERROR_SLL is handled). I believe the yassl error range is 101 to 122.
[21 Feb 2018 17:45] Sinisa Milivojevic
Thank you .....
[30 May 18:05] Vladislavs Sokurenko
Hello, I am experiencing same issue and trying to investigate it on 5.7 and stumbled upon this report, here are my findings, hopefully this will help.

Long story short is that when SIGTERM is received during recv() function then mysql_real_query() will enter infinite loop due to EINTR not being correctly handled the following patch fixes the issue:
{code}
diff --git a/extra/yassl/src/socket_wrapper.cpp b/extra/yassl/src/socket_wrapper.cpp
index a23c1c12e29..0b471bd6d1e 100644
--- a/extra/yassl/src/socket_wrapper.cpp
+++ b/extra/yassl/src/socket_wrapper.cpp
@@ -184,6 +184,7 @@ uint Socket::receive(byte* buf, unsigned int sz)
     // idea to seperate error from would block by arnetheduck@gmail.com
     if (recvd == -1) {
         if (get_lastError() == SOCKET_EWOULDBLOCK || 
+            get_lastError() == EINTR ||
             get_lastError() == SOCKET_EAGAIN) {
             wouldBlock_  = true; // would have blocked this time only
             nonBlocking_ = true; // socket nonblocking, win32 only way to tell
{code}

More details:
114 error code come from, it's handshake.c:761
https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/extra/...
{code}
    // add new data
    uint read  = ssl.useSocket().receive(buffer.get_buffer() + buffSz, ready);
    if (read == static_cast<uint>(-1)) {
        ssl.SetError(receive_error);
        return 0;
{code}

Later however in https://github.com/mysql/mysql-server/blob/124c7ab1d6f914637521fd4463a993aa73403513/vio/vi... there is comment "Set error status to a equivalent of the SSL error"

notice following code:
{code}
if (error)
  {
#ifdef _WIN32
    WSASetLastError(error);
#else
    errno= error;
#endif
  }
{code}

So errno remains EINTR and this leads to following situation:
{code}
my_bool
vio_should_retry(Vio *vio)
{
  return (vio_errno(vio) == SOCKET_EINTR);
}
{code}

Notice that it will retry on interrupt, but when it retry it will always receive 0 and EINTR errno will be always set.

The question is also why does it receive 0 on next runs.

I will update once I have more information
[31 May 7:26] Vladislavs Sokurenko
The answer as to why it receive 0 on next runs is that it does not enter receive in case of interrupt and always exit early here https://github.com/mysql/mysql-server/blob/8cc757da3d87bf4a1f07dcfb2d3c96fed3806870/extra/...
[31 May 9:06] Vladislavs Sokurenko
Attached log with your patch however assert was changed to fprintf(stderr, "ASSERT FAILURE\n"); this is to catch infinite loop

Attachment: yassl_verbose_29-01.log (text/x-log), 2.90 MiB.

[31 May 12:58] Sinisa Milivojevic
Hello Vladislava,

Thank you for your contribution.

We do require some further info from you. 

Does your patch fully resolves the issue ???

Is our patch or any part of it necessary for the full resolution of this issue ????

Thank you very much, in advance.
[31 May 13:24] Vladislavs Sokurenko
Hello  Sinisa, attached EINTR_fix.diff patch is enough to fix the issue, nothing else is required

Attachment: EINTR_fix.diff (text/x-patch), 1.11 KiB.

[31 May 13:40] Sinisa Milivojevic
Thank you very much Mrs. Vladislava !!!

This bug is reverted back to the verified status.

However, we can not use your patch, until you sign OCA agreement with us.

Please read the instructions that you will find below:

In order to submit contributions you must first sign the Oracle
Contribution Agreement (OCA). For additional information please check
http://www.oracle.com/technetwork/community/oca-486395.html

If you have any questions, please contact the MySQL community team at
http://www.mysql.com/about/contact/?topic=community
[31 May 14:52] Sinisa Milivojevic
Vladislava,

Thank you very much .....
[20 Jun 19:42] Vladislavs Sokurenko
fixed mysql_real_query() not to hang when signal is received

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: EINTR_fix.diff (text/x-patch), 1.11 KiB.

[21 Jun 12:29] Sinisa Milivojevic
Dear Vladislava,

Thank you very much for your contribution !!!!

I will make this patch available to the Development.