diff options
-rw-r--r-- | src/ChangeLog | 26 | ||||
-rw-r--r-- | src/buffer.h | 6 | ||||
-rw-r--r-- | src/editfns.c | 27 | ||||
-rw-r--r-- | src/fileio.c | 13 | ||||
-rw-r--r-- | src/insdel.c | 44 | ||||
-rw-r--r-- | src/lisp.h | 1 |
6 files changed, 60 insertions, 57 deletions
diff --git a/src/ChangeLog b/src/ChangeLog index 59fb2d89b24..5f18c8d0062 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,29 @@ +2011-06-16 Paul Eggert <eggert@cs.ucla.edu> + + Improve buffer-overflow checking. + * fileio.c (Finsert_file_contents): + * insdel.c (insert_from_buffer_1, replace_range, replace_range_2): + Remove the old (too-loose) buffer overflow checks. + They weren't needed, since make_gap checks for buffer overflow. + * insdel.c (make_gap_larger): Catch buffer overflows that were missed. + The old code merely checked for Emacs fixnum overflow, and relied + on undefined (wraparound) behavior. The new code avoids undefined + behavior, and also checks for ptrdiff_t and/or size_t overflow. + + * editfns.c (Finsert_char): Don't dump core with very negative counts. + Tune. Don't use wider integers than needed. Don't use alloca. + Use a bigger 'string' buffer. Rewrite to avoid 'n > 0' test. + + * insdel.c (replace_range): Fix buf overflow when insbytes < outgoing. + + * insdel.c, lisp.h (buffer_overflow): New function. + (insert_from_buffer_1, replace_range, replace_range_2): + * insdel.c (make_gap_larger): + * editfns.c (Finsert_char): + * fileio.c (Finsert_file_contents): Use it, to normalize wording. + + * buffer.h (BUF_BYTES_MAX): Cast to ptrdiff_t so that it's signed. + 2011-06-15 Paul Eggert <eggert@cs.ucla.edu> Integer overflow and signedness fixes (Bug#8873). diff --git a/src/buffer.h b/src/buffer.h index dc1d62beb00..a13351b5ea6 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -309,8 +309,10 @@ while (0) /* Maximum number of bytes in a buffer. A buffer cannot contain more bytes than a 1-origin fixnum can represent, - nor can it be so large that C pointer arithmetic stops working. */ -#define BUF_BYTES_MAX min (MOST_POSITIVE_FIXNUM - 1, min (SIZE_MAX, PTRDIFF_MAX)) + nor can it be so large that C pointer arithmetic stops working. + The ptrdiff_t cast ensures that this is signed, not unsigned. */ +#define BUF_BYTES_MAX \ + (ptrdiff_t) min (MOST_POSITIVE_FIXNUM - 1, min (SIZE_MAX, PTRDIFF_MAX)) /* Return the address of byte position N in current buffer. */ diff --git a/src/editfns.c b/src/editfns.c index 9678d4da6a2..2d736bbc7e2 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -2328,12 +2328,11 @@ The optional third arg INHERIT, if non-nil, says to inherit text properties from adjoining text, if those properties are sticky. */) (Lisp_Object character, Lisp_Object count, Lisp_Object inherit) { - register char *string; - register EMACS_INT stringlen; - register int i; + int i, stringlen; register EMACS_INT n; int c, len; unsigned char str[MAX_MULTIBYTE_LENGTH]; + char string[4000]; CHECK_CHARACTER (character); CHECK_NUMBER (count); @@ -2343,16 +2342,15 @@ from adjoining text, if those properties are sticky. */) len = CHAR_STRING (c, str); else str[0] = c, len = 1; + if (XINT (count) <= 0) + return Qnil; if (BUF_BYTES_MAX / len < XINT (count)) - error ("Maximum buffer size would be exceeded"); + buffer_overflow (); n = XINT (count) * len; - if (n <= 0) - return Qnil; - stringlen = min (n, 256 * len); - string = (char *) alloca (stringlen); + stringlen = min (n, sizeof string - sizeof string % len); for (i = 0; i < stringlen; i++) string[i] = str[i % len]; - while (n >= stringlen) + while (n > stringlen) { QUIT; if (!NILP (inherit)) @@ -2361,13 +2359,10 @@ from adjoining text, if those properties are sticky. */) insert (string, stringlen); n -= stringlen; } - if (n > 0) - { - if (!NILP (inherit)) - insert_and_inherit (string, n); - else - insert (string, n); - } + if (!NILP (inherit)) + insert_and_inherit (string, n); + else + insert (string, n); return Qnil; } diff --git a/src/fileio.c b/src/fileio.c index fd5277cbd59..dd34872c263 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -3264,7 +3264,7 @@ variable `last-coding-system-used' to the coding system actually used. */) platform that allows file sizes greater than the maximum off_t value. */ if (! not_regular && ! (0 <= st.st_size && st.st_size <= BUF_BYTES_MAX)) - error ("Maximum buffer size exceeded"); + buffer_overflow (); /* Prevent redisplay optimizations. */ current_buffer->clip_changed = 1; @@ -3800,16 +3800,7 @@ variable `last-coding-system-used' to the coding system actually used. */) } if (! not_regular) - { - register Lisp_Object temp; - - total = XINT (end) - XINT (beg); - - /* Make sure point-max won't overflow after this insertion. */ - XSETINT (temp, total); - if (total != XINT (temp)) - error ("Maximum buffer size exceeded"); - } + total = XINT (end) - XINT (beg); else /* For a special file, all we can do is guess. */ total = READ_BUF_SIZE; diff --git a/src/insdel.c b/src/insdel.c index c0cccc65d6a..bc95e3ad9e8 100644 --- a/src/insdel.c +++ b/src/insdel.c @@ -391,6 +391,12 @@ adjust_markers_for_replace (EMACS_INT from, EMACS_INT from_byte, } +void +buffer_overflow (void) +{ + error ("Maximum buffer size exceeded"); +} + /* Make the gap NBYTES_ADDED bytes longer. */ static void @@ -400,16 +406,16 @@ make_gap_larger (EMACS_INT nbytes_added) EMACS_INT real_gap_loc; EMACS_INT real_gap_loc_byte; EMACS_INT old_gap_size; + EMACS_INT current_size = Z_BYTE - BEG_BYTE + GAP_SIZE; + enum { enough_for_a_while = 2000 }; - /* If we have to get more space, get enough to last a while. */ - nbytes_added += 2000; + if (BUF_BYTES_MAX - current_size < nbytes_added) + buffer_overflow (); - { EMACS_INT total_size = Z_BYTE - BEG_BYTE + GAP_SIZE + nbytes_added; - if (total_size < 0 - /* Don't allow a buffer size that won't fit in a Lisp integer. */ - || total_size != XINT (make_number (total_size))) - error ("Buffer exceeds maximum size"); - } + /* If we have to get more space, get enough to last a while; + but do not exceed the maximum buffer size. */ + nbytes_added = min (nbytes_added + enough_for_a_while, + BUF_BYTES_MAX - current_size); enlarge_buffer_text (current_buffer, nbytes_added); @@ -1063,7 +1069,6 @@ static void insert_from_buffer_1 (struct buffer *buf, EMACS_INT from, EMACS_INT nchars, int inherit) { - register Lisp_Object temp; EMACS_INT chunk, chunk_expanded; EMACS_INT from_byte = buf_charpos_to_bytepos (buf, from); EMACS_INT to_byte = buf_charpos_to_bytepos (buf, from + nchars); @@ -1102,11 +1107,6 @@ insert_from_buffer_1 (struct buffer *buf, outgoing_nbytes = outgoing_before_gap + outgoing_after_gap; } - /* Make sure point-max won't overflow after this insertion. */ - XSETINT (temp, outgoing_nbytes + Z); - if (outgoing_nbytes + Z != XINT (temp)) - error ("Maximum buffer size exceeded"); - /* Do this before moving and increasing the gap, because the before-change hooks might move the gap or make it smaller. */ @@ -1303,7 +1303,6 @@ replace_range (EMACS_INT from, EMACS_INT to, Lisp_Object new, EMACS_INT insbytes = SBYTES (new); EMACS_INT from_byte, to_byte; EMACS_INT nbytes_del, nchars_del; - register Lisp_Object temp; struct gcpro gcpro1; INTERVAL intervals; EMACS_INT outgoing_insbytes = insbytes; @@ -1347,11 +1346,6 @@ replace_range (EMACS_INT from, EMACS_INT to, Lisp_Object new, outgoing_insbytes = count_size_as_multibyte (SDATA (new), insbytes); - /* Make sure point-max won't overflow after this insertion. */ - XSETINT (temp, Z_BYTE - nbytes_del + insbytes); - if (Z_BYTE - nbytes_del + insbytes != XINT (temp)) - error ("Maximum buffer size exceeded"); - GCPRO1 (new); /* Make sure the gap is somewhere in or next to what we are deleting. */ @@ -1383,8 +1377,8 @@ replace_range (EMACS_INT from, EMACS_INT to, Lisp_Object new, if (Z - GPT < END_UNCHANGED) END_UNCHANGED = Z - GPT; - if (GAP_SIZE < insbytes) - make_gap (insbytes - GAP_SIZE); + if (GAP_SIZE < outgoing_insbytes) + make_gap (outgoing_insbytes - GAP_SIZE); /* Copy the string text into the buffer, perhaps converting between single-byte and multibyte. */ @@ -1482,7 +1476,6 @@ replace_range_2 (EMACS_INT from, EMACS_INT from_byte, int markers) { EMACS_INT nbytes_del, nchars_del; - Lisp_Object temp; CHECK_MARKERS (); @@ -1492,11 +1485,6 @@ replace_range_2 (EMACS_INT from, EMACS_INT from_byte, if (nbytes_del <= 0 && insbytes == 0) return; - /* Make sure point-max won't overflow after this insertion. */ - XSETINT (temp, Z_BYTE - nbytes_del + insbytes); - if (Z_BYTE - nbytes_del + insbytes != XINT (temp)) - error ("Maximum buffer size exceeded"); - /* Make sure the gap is somewhere in or next to what we are deleting. */ if (from > GPT) gap_right (from, from_byte); diff --git a/src/lisp.h b/src/lisp.h index 83534e55433..75827deda1a 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -2635,6 +2635,7 @@ extern void init_image (void); extern Lisp_Object Qinhibit_modification_hooks; extern void move_gap (EMACS_INT); extern void move_gap_both (EMACS_INT, EMACS_INT); +extern void buffer_overflow (void) NO_RETURN; extern void make_gap (EMACS_INT); extern EMACS_INT copy_text (const unsigned char *, unsigned char *, EMACS_INT, int, int); |