diff options
| author | Paul Eggert | 2011-04-12 22:02:54 -0700 |
|---|---|---|
| committer | Paul Eggert | 2011-04-12 22:02:54 -0700 |
| commit | 273a5f82856e545365fbf9278bd739cb6c5aa35e (patch) | |
| tree | bd7ce9c14b199db74fd95b29fc97bf07fd633eb9 /src | |
| parent | 3e047f51d5ad36df46d553d1090e28f546af9382 (diff) | |
| download | emacs-273a5f82856e545365fbf9278bd739cb6c5aa35e.tar.gz emacs-273a5f82856e545365fbf9278bd739cb6c5aa35e.zip | |
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.
Diffstat (limited to 'src')
| -rw-r--r-- | src/ChangeLog | 14 | ||||
| -rw-r--r-- | src/gnutls.c | 6 | ||||
| -rw-r--r-- | src/gnutls.h | 2 | ||||
| -rw-r--r-- | src/lisp.h | 2 | ||||
| -rw-r--r-- | src/process.c | 24 | ||||
| -rw-r--r-- | src/sysdep.c | 30 |
6 files changed, 45 insertions, 33 deletions
diff --git a/src/ChangeLog b/src/ChangeLog index 6b0e575e36d..73d27f26d45 100644 --- a/src/ChangeLog +++ b/src/ChangeLog | |||
| @@ -1,5 +1,19 @@ | |||
| 1 | 2011-04-13 Paul Eggert <eggert@cs.ucla.edu> | 1 | 2011-04-13 Paul Eggert <eggert@cs.ucla.edu> |
| 2 | 2 | ||
| 3 | emacs_write: Return size_t, not ssize_t, to avoid overflow issues. | ||
| 4 | * gnutls.c, gnutls.h (emacs_gnutls_write): Return size_t, not ssize_t. | ||
| 5 | * sysdep.c, lisp.h (emacs_write): Likewise. | ||
| 6 | Without the above change, emacs_gnutls_write and emacs_write had | ||
| 7 | undefined behavior and would typically mistakenly report an error | ||
| 8 | when writing a buffer whose size exceeds SSIZE_MAX. | ||
| 9 | (emacs_read, emacs_write): Remove check for negative size, as the | ||
| 10 | Emacs source code has been audited now. | ||
| 11 | (emacs_write): Adjust to new signature, making the code look more | ||
| 12 | like that of emacs_gnutls_write. | ||
| 13 | * process.c (send_process): Adjust to the new signatures of | ||
| 14 | emacs_write and emacs_gnutls_write. Do not attempt to store | ||
| 15 | a byte offset into an 'int'; it might overflow. | ||
| 16 | |||
| 3 | * sound.c: Don't assume sizes fit in 'int'. | 17 | * sound.c: Don't assume sizes fit in 'int'. |
| 4 | (struct sound_device.period_size, alsa_period_size): | 18 | (struct sound_device.period_size, alsa_period_size): |
| 5 | Return size_t, not int. | 19 | Return size_t, not int. |
diff --git a/src/gnutls.c b/src/gnutls.c index d9e4dcec15a..2974f048459 100644 --- a/src/gnutls.c +++ b/src/gnutls.c | |||
| @@ -70,7 +70,7 @@ emacs_gnutls_handshake (struct Lisp_Process *proc) | |||
| 70 | } | 70 | } |
| 71 | } | 71 | } |
| 72 | 72 | ||
| 73 | ssize_t | 73 | size_t |
| 74 | emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, | 74 | emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, |
| 75 | size_t nbyte) | 75 | size_t nbyte) |
| 76 | { | 76 | { |
| @@ -85,7 +85,7 @@ emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, | |||
| 85 | #ifdef EAGAIN | 85 | #ifdef EAGAIN |
| 86 | errno = EAGAIN; | 86 | errno = EAGAIN; |
| 87 | #endif | 87 | #endif |
| 88 | return -1; | 88 | return 0; |
| 89 | } | 89 | } |
| 90 | 90 | ||
| 91 | bytes_written = 0; | 91 | bytes_written = 0; |
| @@ -99,7 +99,7 @@ emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, | |||
| 99 | if (rtnval == GNUTLS_E_AGAIN || rtnval == GNUTLS_E_INTERRUPTED) | 99 | if (rtnval == GNUTLS_E_AGAIN || rtnval == GNUTLS_E_INTERRUPTED) |
| 100 | continue; | 100 | continue; |
| 101 | else | 101 | else |
| 102 | return (bytes_written ? bytes_written : -1); | 102 | break; |
| 103 | } | 103 | } |
| 104 | 104 | ||
| 105 | buf += rtnval; | 105 | buf += rtnval; |
diff --git a/src/gnutls.h b/src/gnutls.h index b39131b6236..11f681f9c7b 100644 --- a/src/gnutls.h +++ b/src/gnutls.h | |||
| @@ -50,7 +50,7 @@ typedef enum | |||
| 50 | 50 | ||
| 51 | #define GNUTLS_LOG2(level, max, string, extra) if (level <= max) { gnutls_log_function2 (level, "(Emacs) " string, extra); } | 51 | #define GNUTLS_LOG2(level, max, string, extra) if (level <= max) { gnutls_log_function2 (level, "(Emacs) " string, extra); } |
| 52 | 52 | ||
| 53 | ssize_t | 53 | size_t |
| 54 | emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, | 54 | emacs_gnutls_write (int fildes, struct Lisp_Process *proc, const char *buf, |
| 55 | size_t nbyte); | 55 | size_t nbyte); |
| 56 | ssize_t | 56 | ssize_t |
diff --git a/src/lisp.h b/src/lisp.h index 080b2693a41..ae8f3c793c5 100644 --- a/src/lisp.h +++ b/src/lisp.h | |||
| @@ -3347,7 +3347,7 @@ extern void seed_random (long); | |||
| 3347 | extern int emacs_open (const char *, int, int); | 3347 | extern int emacs_open (const char *, int, int); |
| 3348 | extern int emacs_close (int); | 3348 | extern int emacs_close (int); |
| 3349 | extern ssize_t emacs_read (int, char *, size_t); | 3349 | extern ssize_t emacs_read (int, char *, size_t); |
| 3350 | extern ssize_t emacs_write (int, const char *, size_t); | 3350 | extern size_t emacs_write (int, const char *, size_t); |
| 3351 | enum { READLINK_BUFSIZE = 1024 }; | 3351 | enum { READLINK_BUFSIZE = 1024 }; |
| 3352 | extern char *emacs_readlink (const char *, char [READLINK_BUFSIZE]); | 3352 | extern char *emacs_readlink (const char *, char [READLINK_BUFSIZE]); |
| 3353 | #ifndef HAVE_MEMSET | 3353 | #ifndef HAVE_MEMSET |
diff --git a/src/process.c b/src/process.c index 624610069d8..2eed7b4654f 100644 --- a/src/process.c +++ b/src/process.c | |||
| @@ -5367,6 +5367,7 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, | |||
| 5367 | /* Send this batch, using one or more write calls. */ | 5367 | /* Send this batch, using one or more write calls. */ |
| 5368 | while (this > 0) | 5368 | while (this > 0) |
| 5369 | { | 5369 | { |
| 5370 | size_t written = 0; | ||
| 5370 | int outfd = p->outfd; | 5371 | int outfd = p->outfd; |
| 5371 | old_sigpipe = (void (*) (int)) signal (SIGPIPE, send_process_trap); | 5372 | old_sigpipe = (void (*) (int)) signal (SIGPIPE, send_process_trap); |
| 5372 | #ifdef DATAGRAM_SOCKETS | 5373 | #ifdef DATAGRAM_SOCKETS |
| @@ -5375,7 +5376,9 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, | |||
| 5375 | rv = sendto (outfd, buf, this, | 5376 | rv = sendto (outfd, buf, this, |
| 5376 | 0, datagram_address[outfd].sa, | 5377 | 0, datagram_address[outfd].sa, |
| 5377 | datagram_address[outfd].len); | 5378 | datagram_address[outfd].len); |
| 5378 | if (rv < 0 && errno == EMSGSIZE) | 5379 | if (0 <= rv) |
| 5380 | written = rv; | ||
| 5381 | else if (errno == EMSGSIZE) | ||
| 5379 | { | 5382 | { |
| 5380 | signal (SIGPIPE, old_sigpipe); | 5383 | signal (SIGPIPE, old_sigpipe); |
| 5381 | report_file_error ("sending datagram", | 5384 | report_file_error ("sending datagram", |
| @@ -5387,12 +5390,13 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, | |||
| 5387 | { | 5390 | { |
| 5388 | #ifdef HAVE_GNUTLS | 5391 | #ifdef HAVE_GNUTLS |
| 5389 | if (XPROCESS (proc)->gnutls_p) | 5392 | if (XPROCESS (proc)->gnutls_p) |
| 5390 | rv = emacs_gnutls_write (outfd, | 5393 | written = emacs_gnutls_write (outfd, |
| 5391 | XPROCESS (proc), | 5394 | XPROCESS (proc), |
| 5392 | buf, this); | 5395 | buf, this); |
| 5393 | else | 5396 | else |
| 5394 | #endif | 5397 | #endif |
| 5395 | rv = emacs_write (outfd, buf, this); | 5398 | written = emacs_write (outfd, buf, this); |
| 5399 | rv = (written == this ? 0 : -1); | ||
| 5396 | #ifdef ADAPTIVE_READ_BUFFERING | 5400 | #ifdef ADAPTIVE_READ_BUFFERING |
| 5397 | if (p->read_output_delay > 0 | 5401 | if (p->read_output_delay > 0 |
| 5398 | && p->adaptive_read_buffering == 1) | 5402 | && p->adaptive_read_buffering == 1) |
| @@ -5419,7 +5423,7 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, | |||
| 5419 | that may allow the program | 5423 | that may allow the program |
| 5420 | to finish doing output and read more. */ | 5424 | to finish doing output and read more. */ |
| 5421 | { | 5425 | { |
| 5422 | int offset = 0; | 5426 | size_t offset = 0; |
| 5423 | 5427 | ||
| 5424 | #ifdef BROKEN_PTY_READ_AFTER_EAGAIN | 5428 | #ifdef BROKEN_PTY_READ_AFTER_EAGAIN |
| 5425 | /* A gross hack to work around a bug in FreeBSD. | 5429 | /* A gross hack to work around a bug in FreeBSD. |
| @@ -5465,16 +5469,14 @@ send_process (volatile Lisp_Object proc, const char *volatile buf, | |||
| 5465 | offset); | 5469 | offset); |
| 5466 | else if (STRINGP (object)) | 5470 | else if (STRINGP (object)) |
| 5467 | buf = offset + SSDATA (object); | 5471 | buf = offset + SSDATA (object); |
| 5468 | |||
| 5469 | rv = 0; | ||
| 5470 | } | 5472 | } |
| 5471 | else | 5473 | else |
| 5472 | /* This is a real error. */ | 5474 | /* This is a real error. */ |
| 5473 | report_file_error ("writing to process", Fcons (proc, Qnil)); | 5475 | report_file_error ("writing to process", Fcons (proc, Qnil)); |
| 5474 | } | 5476 | } |
| 5475 | buf += rv; | 5477 | buf += written; |
| 5476 | len -= rv; | 5478 | len -= written; |
| 5477 | this -= rv; | 5479 | this -= written; |
| 5478 | } | 5480 | } |
| 5479 | } | 5481 | } |
| 5480 | } | 5482 | } |
diff --git a/src/sysdep.c b/src/sysdep.c index d56e2a864dc..84c8d4ec0ea 100644 --- a/src/sysdep.c +++ b/src/sysdep.c | |||
| @@ -1825,41 +1825,36 @@ emacs_close (int fd) | |||
| 1825 | return rtnval; | 1825 | return rtnval; |
| 1826 | } | 1826 | } |
| 1827 | 1827 | ||
| 1828 | /* Read from FILEDESC to a buffer BUF with size NBYTE, retrying if interrupted. | ||
| 1829 | Return the number of bytes read, which might be less than NBYTE. | ||
| 1830 | On error, set errno and return -1. */ | ||
| 1828 | ssize_t | 1831 | ssize_t |
| 1829 | emacs_read (int fildes, char *buf, size_t nbyte) | 1832 | emacs_read (int fildes, char *buf, size_t nbyte) |
| 1830 | { | 1833 | { |
| 1831 | register ssize_t rtnval; | 1834 | register ssize_t rtnval; |
| 1832 | 1835 | ||
| 1833 | /* Defend against the possibility that a buggy caller passes a negative NBYTE | ||
| 1834 | argument, which would be converted to a large unsigned size_t NBYTE. This | ||
| 1835 | defense prevents callers from doing large writes, unfortunately. This | ||
| 1836 | size restriction can be removed once we have carefully checked that there | ||
| 1837 | are no such callers. */ | ||
| 1838 | if ((ssize_t) nbyte < 0) | ||
| 1839 | abort (); | ||
| 1840 | |||
| 1841 | while ((rtnval = read (fildes, buf, nbyte)) == -1 | 1836 | while ((rtnval = read (fildes, buf, nbyte)) == -1 |
| 1842 | && (errno == EINTR)) | 1837 | && (errno == EINTR)) |
| 1843 | QUIT; | 1838 | QUIT; |
| 1844 | return (rtnval); | 1839 | return (rtnval); |
| 1845 | } | 1840 | } |
| 1846 | 1841 | ||
| 1847 | ssize_t | 1842 | /* Write to FILEDES from a buffer BUF with size NBYTE, retrying if interrupted |
| 1843 | or if a partial write occurs. Return the number of bytes written, setting | ||
| 1844 | errno if this is less than NBYTE. */ | ||
| 1845 | size_t | ||
| 1848 | emacs_write (int fildes, const char *buf, size_t nbyte) | 1846 | emacs_write (int fildes, const char *buf, size_t nbyte) |
| 1849 | { | 1847 | { |
| 1850 | register ssize_t rtnval, bytes_written; | 1848 | ssize_t rtnval; |
| 1851 | 1849 | size_t bytes_written; | |
| 1852 | /* Defend against negative NBYTE, as in emacs_read. */ | ||
| 1853 | if ((ssize_t) nbyte < 0) | ||
| 1854 | abort (); | ||
| 1855 | 1850 | ||
| 1856 | bytes_written = 0; | 1851 | bytes_written = 0; |
| 1857 | 1852 | ||
| 1858 | while (nbyte != 0) | 1853 | while (nbyte > 0) |
| 1859 | { | 1854 | { |
| 1860 | rtnval = write (fildes, buf, nbyte); | 1855 | rtnval = write (fildes, buf, nbyte); |
| 1861 | 1856 | ||
| 1862 | if (rtnval == -1) | 1857 | if (rtnval < 0) |
| 1863 | { | 1858 | { |
| 1864 | if (errno == EINTR) | 1859 | if (errno == EINTR) |
| 1865 | { | 1860 | { |
| @@ -1871,13 +1866,14 @@ emacs_write (int fildes, const char *buf, size_t nbyte) | |||
| 1871 | continue; | 1866 | continue; |
| 1872 | } | 1867 | } |
| 1873 | else | 1868 | else |
| 1874 | return (bytes_written ? bytes_written : -1); | 1869 | break; |
| 1875 | } | 1870 | } |
| 1876 | 1871 | ||
| 1877 | buf += rtnval; | 1872 | buf += rtnval; |
| 1878 | nbyte -= rtnval; | 1873 | nbyte -= rtnval; |
| 1879 | bytes_written += rtnval; | 1874 | bytes_written += rtnval; |
| 1880 | } | 1875 | } |
| 1876 | |||
| 1881 | return (bytes_written); | 1877 | return (bytes_written); |
| 1882 | } | 1878 | } |
| 1883 | 1879 | ||