From: Paul Eggert Date: Wed, 16 Jan 2019 07:51:45 +0000 (-0800) Subject: Fix unlikely races with GnuTLS, datagrams X-Git-Tag: emacs-27.0.90~3816^2~2 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=e87e6a24c49542111e669b7d0f1a412024663f8e;p=emacs.git 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 d0cb28dc536..63dbcf4162b 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -72,7 +72,7 @@ along with GNU Emacs. If not, see . */ # 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 06555bac4c0..c0741403b57 100644 --- 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)