From 273a5f82856e545365fbf9278bd739cb6c5aa35e Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 12 Apr 2011 22:02:54 -0700 Subject: [PATCH] emacs_write: Return size_t, not ssize_t, to avoid overflow issues. * gnutls.c, gnutls.h (emacs_gnutls_write): Return size_t, not ssize_t. * sysdep.c, lisp.h (emacs_write): Likewise. Without the above change, emacs_gnutls_write and emacs_write had undefined behavior and would typically mistakenly report an error when writing a buffer whose size exceeds SSIZE_MAX. (emacs_read, emacs_write): Remove check for negative size, as the Emacs source code has been audited now. (emacs_write): Adjust to new signature, making the code look more like that of emacs_gnutls_write. * process.c (send_process): Adjust to the new signatures of emacs_write and emacs_gnutls_write. Do not attempt to store a byte offset into an 'int'; it might overflow. --- src/ChangeLog | 14 ++++++++++++++ src/gnutls.c | 6 +++--- src/gnutls.h | 2 +- src/lisp.h | 2 +- src/process.c | 24 +++++++++++++----------- src/sysdep.c | 30 +++++++++++++----------------- 6 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 6b0e575e36..73d27f26d4 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,19 @@ 2011-04-13 Paul Eggert + emacs_write: Return size_t, not ssize_t, to avoid overflow issues. + * gnutls.c, gnutls.h (emacs_gnutls_write): Return size_t, not ssize_t. + * sysdep.c, lisp.h (emacs_write): Likewise. + Without the above change, emacs_gnutls_write and emacs_write had + undefined behavior and would typically mistakenly report an error + when writing a buffer whose size exceeds SSIZE_MAX. + (emacs_read, emacs_write): Remove check for negative size, as the + Emacs source code has been audited now. + (emacs_write): Adjust to new signature, making the code look more + like that of emacs_gnutls_write. + * process.c (send_process): Adjust to the new signatures of + emacs_write and emacs_gnutls_write. Do not attempt to store + a byte offset into an 'int'; it might overflow. + * sound.c: Don't assume sizes fit in 'int'. (struct sound_device.period_size, alsa_period_size): Return size_t, not int. diff --git a/src/gnutls.c b/src/gnutls.c index d9e4dcec15..2974f04845 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -70,7 +70,7 @@ emacs_gnutls_handshake (struct Lisp_Process *proc) } } -ssize_t +size_t emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, size_t nbyte) { @@ -85,7 +85,7 @@ emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, #ifdef EAGAIN errno = EAGAIN; #endif - return -1; + return 0; } bytes_written = 0; @@ -99,7 +99,7 @@ emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, if (rtnval == GNUTLS_E_AGAIN || rtnval == GNUTLS_E_INTERRUPTED) continue; else - return (bytes_written ? bytes_written : -1); + break; } buf += rtnval; diff --git a/src/gnutls.h b/src/gnutls.h index b39131b623..11f681f9c7 100644 --- a/src/gnutls.h +++ b/src/gnutls.h @@ -50,7 +50,7 @@ typedef enum #define GNUTLS_LOG2(level, max, string, extra) if (level <= max) { gnutls_log_function2 (level, "(Emacs) " string, extra); } -ssize_t +size_t emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, size_t nbyte); ssize_t diff --git a/src/lisp.h b/src/lisp.h index 080b2693a4..ae8f3c793c 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3347,7 +3347,7 @@ extern void seed_random (long); extern int emacs_open (const char *, int, int); extern int emacs_close (int); extern ssize_t emacs_read (int, char *, size_t); -extern ssize_t emacs_write (int, const char *, size_t); +extern size_t emacs_write (int, const char *, size_t); enum { READLINK_BUFSIZE = 1024 }; extern char *emacs_readlink (const char *, char [READLINK_BUFSIZE]); #ifndef HAVE_MEMSET diff --git a/src/process.c b/src/process.c index 624610069d..2eed7b4654 100644 --- a/src/process.c +++ b/src/process.c @@ -5367,6 +5367,7 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, /* Send this batch, using one or more write calls. */ while (this > 0) { + size_t written = 0; int outfd = p->outfd; old_sigpipe = (void (*) (int)) signal (SIGPIPE, send_process_trap); #ifdef DATAGRAM_SOCKETS @@ -5375,7 +5376,9 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, rv = sendto (outfd, buf, this, 0, datagram_address[outfd].sa, datagram_address[outfd].len); - if (rv < 0 && errno == EMSGSIZE) + if (0 <= rv) + written = rv; + else if (errno == EMSGSIZE) { signal (SIGPIPE, old_sigpipe); report_file_error ("sending datagram", @@ -5387,12 +5390,13 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, { #ifdef HAVE_GNUTLS if (XPROCESS (proc)->gnutls_p) - rv = emacs_gnutls_write (outfd, - XPROCESS (proc), - buf, this); + written = emacs_gnutls_write (outfd, + XPROCESS (proc), + buf, this); else #endif - rv = emacs_write (outfd, buf, this); + written = emacs_write (outfd, buf, this); + rv = (written == this ? 0 : -1); #ifdef ADAPTIVE_READ_BUFFERING if (p->read_output_delay > 0 && p->adaptive_read_buffering == 1) @@ -5419,7 +5423,7 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, that may allow the program to finish doing output and read more. */ { - int offset = 0; + size_t offset = 0; #ifdef BROKEN_PTY_READ_AFTER_EAGAIN /* A gross hack to work around a bug in FreeBSD. @@ -5465,16 +5469,14 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, offset); else if (STRINGP (object)) buf = offset + SSDATA (object); - - rv = 0; } else /* This is a real error. */ report_file_error ("writing to process", Fcons (proc, Qnil)); } - buf += rv; - len -= rv; - this -= rv; + buf += written; + len -= written; + this -= written; } } } diff --git a/src/sysdep.c b/src/sysdep.c index d56e2a864d..84c8d4ec0e 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -1825,41 +1825,36 @@ emacs_close (int fd) return rtnval; } +/* Read from FILEDESC to a buffer BUF with size NBYTE, retrying if interrupted. + Return the number of bytes read, which might be less than NBYTE. + On error, set errno and return -1. */ ssize_t emacs_read (int fildes, char *buf, size_t nbyte) { register ssize_t rtnval; - /* Defend against the possibility that a buggy caller passes a negative NBYTE - argument, which would be converted to a large unsigned size_t NBYTE. This - defense prevents callers from doing large writes, unfortunately. This - size restriction can be removed once we have carefully checked that there - are no such callers. */ - if ((ssize_t) nbyte < 0) - abort (); - while ((rtnval = read (fildes, buf, nbyte)) == -1 && (errno == EINTR)) QUIT; return (rtnval); } -ssize_t +/* Write to FILEDES from a buffer BUF with size NBYTE, retrying if interrupted + or if a partial write occurs. Return the number of bytes written, setting + errno if this is less than NBYTE. */ +size_t emacs_write (int fildes, const char *buf, size_t nbyte) { - register ssize_t rtnval, bytes_written; - - /* Defend against negative NBYTE, as in emacs_read. */ - if ((ssize_t) nbyte < 0) - abort (); + ssize_t rtnval; + size_t bytes_written; bytes_written = 0; - while (nbyte != 0) + while (nbyte > 0) { rtnval = write (fildes, buf, nbyte); - if (rtnval == -1) + if (rtnval < 0) { if (errno == EINTR) { @@ -1871,13 +1866,14 @@ emacs_write (int fildes, const char *buf, size_t nbyte) continue; } else - return (bytes_written ? bytes_written : -1); + break; } buf += rtnval; nbyte -= rtnval; bytes_written += rtnval; } + return (bytes_written); } -- 2.20.1