Fix unlikely races with GnuTLS, datagrams

Retry some calls if interrupted at inopportune times.
These were found by code inspection.
* src/gnutls.c (gnutls_try_handshake): Simplify by using
new emacs_gnutls_handle_error API.
(emacs_gnutls_write): Remove GNUTLS_E_AGAIN hack since
emacs_gnutls_handle_error now does that.
Use emacs_gnutls_handle_error only on errors.
(emacs_gnutls_read): Retry if gnutls_record_recv returns
GNUTLS_E_INTERRUPTED, to be consistent with emacs_read.
(emacs_gnutls_handle_error): Return 0 on fatal errors,
-1 (setting errno) on ordinary errors, to simplify callers.
Assume that ERR is negative, since it always is now.
Map non-fatal GnuTLS errors to errno values as best we can.
* src/process.c (read_process_output) [DATAGRAM_SOCKETS]:
Retry recvfrom if it is interrupted, to be consistent with
how things are handled when not a datagram channel.
(send_process) [DATAGRAM_SOCEKTS]: If sendto is interrupted,
process pending signals and retry it, to be consistent with
how things are handled when not a datagram channel.

diff --git a/src/gnutls.c b/src/gnutls.c
index d0cb28dc53609460f6143c89204cd55e00c5b4aa..63dbcf4162b7722c9a8b966fb0055ed42b90877d 100644 (file)
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -72,7 +72,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #  include "w32.h"
 # endif
 
-static bool emacs_gnutls_handle_error (gnutls_session_t, int);
+static int emacs_gnutls_handle_error (gnutls_session_t, int);
 
 static bool gnutls_global_initialized;
 
@@ -579,15 +579,17 @@ gnutls_try_handshake (struct Lisp_Process *proc)
   if (non_blocking)
     proc->gnutls_p = true;
 
-  do
+  while ((ret = gnutls_handshake (state)) < 0)
     {
-      ret = gnutls_handshake (state);
-      emacs_gnutls_handle_error (state, ret);
+      do
+       ret = gnutls_handshake (state);
+      while (ret == GNUTLS_E_INTERRUPTED);
+
+      if (0 <= ret || emacs_gnutls_handle_error (state, ret) == 0
+         || non_blocking)
+       break;
       maybe_quit ();
     }
-  while (ret < 0
-        && gnutls_error_is_fatal (ret) == 0
-        && ! non_blocking);
 
   proc->gnutls_initstage = GNUTLS_STAGE_HANDSHAKE_TRIED;
 
@@ -682,8 +684,6 @@ emacs_gnutls_transport_set_errno (gnutls_session_t state, int err)
 ptrdiff_t
 emacs_gnutls_write (struct Lisp_Process *proc, const char *buf, ptrdiff_t nbyte)
 {
-  ssize_t rtnval = 0;
-  ptrdiff_t bytes_written;
   gnutls_session_t state = proc->gnutls_state;
 
   if (proc->gnutls_initstage != GNUTLS_STAGE_READY)
@@ -692,25 +692,19 @@ emacs_gnutls_write (struct Lisp_Process *proc, const char *buf, ptrdiff_t nbyte)
       return 0;
     }
 
-  bytes_written = 0;
+  ptrdiff_t bytes_written = 0;
 
   while (nbyte > 0)
     {
-      rtnval = gnutls_record_send (state, buf, nbyte);
+      ssize_t rtnval;
+      do
+       rtnval = gnutls_record_send (state, buf, nbyte);
+      while (rtnval == GNUTLS_E_INTERRUPTED);
 
       if (rtnval < 0)
        {
-         if (rtnval == GNUTLS_E_INTERRUPTED)
-           continue;
-         else
-           {
-             /* If we get GNUTLS_E_AGAIN, then set errno
-                appropriately so that send_process retries the
-                correct way instead of erroring out. */
-             if (rtnval == GNUTLS_E_AGAIN)
-               errno = EAGAIN;
-             break;
-           }
+         emacs_gnutls_handle_error (state, rtnval);
+         break;
        }
 
       buf += rtnval;
@@ -718,14 +712,12 @@ emacs_gnutls_write (struct Lisp_Process *proc, const char *buf, ptrdiff_t nbyte)
       bytes_written += rtnval;
     }
 
-  emacs_gnutls_handle_error (state, rtnval);
   return (bytes_written);
 }
 
 ptrdiff_t
 emacs_gnutls_read (struct Lisp_Process *proc, char *buf, ptrdiff_t nbyte)
 {
-  ssize_t rtnval;
   gnutls_session_t state = proc->gnutls_state;
 
   if (proc->gnutls_initstage != GNUTLS_STAGE_READY)
@@ -734,19 +726,18 @@ emacs_gnutls_read (struct Lisp_Process *proc, char *buf, ptrdiff_t nbyte)
       return -1;
     }
 
-  rtnval = gnutls_record_recv (state, buf, nbyte);
+  ssize_t rtnval;
+  do
+    rtnval = gnutls_record_recv (state, buf, nbyte);
+  while (rtnval == GNUTLS_E_INTERRUPTED);
+
   if (rtnval >= 0)
     return rtnval;
   else if (rtnval == GNUTLS_E_UNEXPECTED_PACKET_LENGTH)
     /* The peer closed the connection. */
     return 0;
-  else if (emacs_gnutls_handle_error (state, rtnval))
-    /* non-fatal error */
-    return -1;
-  else {
-    /* a fatal error occurred */
-    return 0;
-  }
+  else
+    return emacs_gnutls_handle_error (state, rtnval);
 }
 
 static char const *
@@ -757,25 +748,24 @@ emacs_gnutls_strerror (int err)
 }
 
 /* Report a GnuTLS error to the user.
-   Return true if the error code was successfully handled.  */
-static bool
+   SESSION is the GnuTLS session, ERR is the (negative) GnuTLS error code.
+   Return 0 if the error was fatal, -1 (setting errno) otherwise so
+   that the caller can notice the error and attempt a repair.  */
+static int
 emacs_gnutls_handle_error (gnutls_session_t session, int err)
 {
-  int max_log_level = 0;
-
-  bool ret;
+  int ret;
 
   /* TODO: use a Lisp_Object generated by gnutls_make_error?  */
-  if (err >= 0)
-    return 1;
 
   check_memory_full (err);
 
-  max_log_level = global_gnutls_log_level;
+  int max_log_level = global_gnutls_log_level;
 
   /* TODO: use gnutls-error-fatalp and gnutls-error-string.  */
 
   char const *str = emacs_gnutls_strerror (err);
+  int errnum = EINVAL;
 
   if (gnutls_error_is_fatal (err))
     {
@@ -789,11 +779,11 @@ emacs_gnutls_handle_error (gnutls_session_t session, int err)
 # endif
 
       GNUTLS_LOG2 (level, max_log_level, "fatal error:", str);
-      ret = false;
+      ret = 0;
     }
   else
     {
-      ret = true;
+      ret = -1;
 
       switch (err)
         {
@@ -809,6 +799,26 @@ emacs_gnutls_handle_error (gnutls_session_t session, int err)
                        "non-fatal error:",
                        str);
         }
+
+      switch (err)
+       {
+       case GNUTLS_E_AGAIN:
+         errnum = EAGAIN;
+         break;
+
+# ifdef EMSGSIZE
+       case GNUTLS_E_LARGE_PACKET:
+       case GNUTLS_E_PUSH_ERROR:
+         errnum = EMSGSIZE;
+         break;
+# endif
+
+# if defined HAVE_GNUTLS3 && defined ECONNRESET
+       case GNUTLS_E_PREMATURE_TERMINATION:
+         errnum = ECONNRESET;
+         break;
+# endif
+       }
     }
 
   if (err == GNUTLS_E_WARNING_ALERT_RECEIVED
@@ -822,6 +832,8 @@ emacs_gnutls_handle_error (gnutls_session_t session, int err)
 
       GNUTLS_LOG2 (level, max_log_level, "Received alert: ", str);
     }
+
+  errno = errnum;
   return ret;
 }
 

diff --git a/src/process.c b/src/process.c
index 06555bac4c080ca9325a2119209033c392e8563c..c0741403b576e269d905c03ea22011a3e06c10c1 100644 (file)
--- a/src/process.c
+++ b/src/process.c
@@ -5840,7 +5840,8 @@ read_and_dispose_of_process_output (struct Lisp_Process *p, char *chars,
 
 /* Read pending output from the process channel,
    starting with our buffered-ahead character if we have one.
-   Yield number of decoded characters read.
+   Yield number of decoded characters read,
+   or -1 (setting errno) if there is a read error.
 
    This function reads at most 4096 characters.
    If you want to read all available subprocess output,
@@ -5870,8 +5871,10 @@ read_process_output (Lisp_Object proc, int channel)
   if (DATAGRAM_CHAN_P (channel))
     {
       socklen_t len = datagram_address[channel].len;
-      nbytes = recvfrom (channel, chars + carryover, readmax,
-                        0, datagram_address[channel].sa, &len);
+      do
+       nbytes = recvfrom (channel, chars + carryover, readmax,
+                          0, datagram_address[channel].sa, &len);
+      while (nbytes < 0 && errno == EINTR);
     }
   else
 #endif
@@ -5921,8 +5924,6 @@ read_process_output (Lisp_Object proc, int channel)
 
   p->decoding_carryover = 0;
 
-  /* At this point, NBYTES holds number of bytes just received
-     (including the one in proc_buffered_char[channel]).  */
   if (nbytes <= 0)
     {
       if (nbytes < 0 || coding->mode & CODING_MODE_LAST_BLOCK)
@@ -5930,6 +5931,9 @@ read_process_output (Lisp_Object proc, int channel)
       coding->mode |= CODING_MODE_LAST_BLOCK;
     }
 
+  /* At this point, NBYTES holds number of bytes just received
+     (including the one in proc_buffered_char[channel]).  */
+
   /* Ignore carryover, it's been added by a previous iteration already.  */
   p->nbytes_read += nbytes;
 
@@ -6372,9 +6376,17 @@ send_process (Lisp_Object proc, const char *buf, ptrdiff_t len,
 #ifdef DATAGRAM_SOCKETS
          if (DATAGRAM_CHAN_P (outfd))
            {
-             rv = sendto (outfd, cur_buf, cur_len,
-                          0, datagram_address[outfd].sa,
-                          datagram_address[outfd].len);
+             while (true)
+               {
+                 rv = sendto (outfd, cur_buf, cur_len, 0,
+                              datagram_address[outfd].sa,
+                              datagram_address[outfd].len);
+                 if (! (rv < 0 && errno == EINTR))
+                   break;
+                 if (pending_signals)
+                   process_pending_signals ();
+               }
+
              if (rv >= 0)
                written = rv;
              else if (errno == EMSGSIZE)