* bug#9874: Fixes for several integer overflow and width issues
@ 2011-10-26 7:51 Paul Eggert
2011-10-26 8:04 ` Andreas Schwab
2011-10-26 14:09 ` Stefan Monnier
0 siblings, 2 replies; 12+ messages in thread
From: Paul Eggert @ 2011-10-26 7:51 UTC (permalink / raw)
To: 9874
[-- Attachment #1: Type: text/plain, Size: 10446 bytes --]
Package: Emacs
Version: 24.0.90
Tags: patch
Attached is a patch to the Emacs trunk (bzr 106194) to fix several
issues, mostly due to integer overflow. This patch was derived by
looking for and fixing common integer-width issues. The patch does
not attempt to fix all these issues, just some issues that are clear
and for which the fixes are straightforward.
The patch only fixes problems; it does not add features. As patches
go, it is relatively large (647 kB) so I have compressed it; if you
want an uncompressed copy please let me know. I would like to install
it after some more testing. If you'd like to wait until after the
next Emacs pretest, please let me know.
Here is a list of issues fixed. This list is not complete, but it
covers the major issues and several of the minor ones. Documenting
the rest of the minor issues would be a big undertaking with a small
payoff.
* Make Emacs safer by using wider ints when narrower ones might overflow.
This fixes many bugs. Here is a partial list. Unless otherwise
specified the bug examples are on typical 64-bit hosts.
- On my Fedora 14 x86-64 host, (signal-process 4294967295 1)
crashes my entire login session, Emacs included, and leaves my
workstation in a corrupted state in which the screen
continually flashes a nonsense pattern and I cannot log in.
This bug occurs because Emacs incorrectly assumes that fixnums
fit into pid_t values, which is not true on typical 64-bit hosts.
- Conversely, Emacs assumes that uid_t, gid_t, pid_t, and file
descriptor values fit into fixnums, but this is not true for
many 32-bit hosts. For example, HP-UX 11i v3 process-IDs can
go as high as 2**30 - 1. This problem affects many primitives,
including call-process and emacs-pid.
- The following code makes Emacs dump core:
(progn
(setq code-conversion-map-vector 0)
(register-code-conversion-map 'x (make-vector 1 1)))
- (font-get-glyphs FONT-OBJECT FROM TO) goes beserk if TO - FROM
exceeds 2**31, and if you're lucky it dumps core.
- A selective-display value of 4294967297 is silently treated as
if it were 1. There is a similar problem with x-max-tooltip-size.
- (char-category-set 4294967296) returns a nonzero value. It
should report an error, since the argument is not a character.
- (char-resolve-modifiers most-negative-fixnum) returns 0; it
should return its argument.
- (unibyte-string 4294967296) does not report an out-of-range
error, as it should.
- On typical 32-bit hosts, define-charset-internal mishandles some cases
where the maximum code is greater than 2**31 (which is true for
some Asian codes). It calculates the wrong value for maximum
codes, reports the wrong ranges when signalling an error, in
some cases does not report values that are out of range, and
(when --with-wide-int is specified and certain compiler
optimization flags are used) it rejects some valid codes.
- (progn (put 'foo 'char-table-extra-slots 4294967296)
(make-char-table 'foo)) does not signal an out-of-range error,
as it should.
- (set-char-table-range CHARSET 4294967296 VALUE) does not report
an out-of-range error, as it should.
- (backward-char most-negative-fixnum) incorrectly behaves as if it were
(backward-char most-positive-fixnum).
- (unencodable-char-position START END CODING-SYSTEM COUNT "abc")
incorrectly treats a START or END or COUNT value like
4294967297 as if it were 1. There are similar problems with
the START and END values of (check-coding-systems-region START
END CODING-SYSTEM-LIST).
- (find-operation-coding-system 'write-region 1 2) has an
off-by-one error that causes it to access the garbage that
is one past its argument array.
- (define-coding-system-internal ...) mishandles many values
outside of C 'int' range: typically it silently converts them
to 'int', resulting in undefined behavior.
- (string-to-number "10" 4294967298) returns the mathematically
incorrect value 2 rather than reporting a range error.
- dbus-call-method omits integer overflow checks in several cases.
- dbus-call-method-asynchronously mishandles large timeouts.
- Long time durations (either positive or negative) are
mishandled. For example, (sleep-for -2147483649.0) waits for
2**31 seconds, whereas it should either fail or return right
away. There are similar bugs in sit-for, read-char,
read-event, read-char-exclusive, accept-process-output, and
hourglass-delay.
- (signal-process (emacs-pid) 4294967296) should fail, because
there is no signal 4294967296, but it returns 0, indicating
success.
- On typical 64-bit hosts with 32-bit uid_t, (user-login-name
4294967296) returns "root" whereas it should fail. There is a
similar problem with (user-full-name 4294967296).
- (float-time '(0 0 4294967296)) returns 0.0, which is incorrect.
It should either report an error for an out-of-range
microseconds component, or return the mathematically-correct
result.
- (encode-time 0 0 0 1 1 1970 4294967296) acts like (encode-time
0 0 0 1 1 1970 0), ignoring the high order bits of the time
zone argument. These high order bits should be passed to the
underlying C implementation (which may well mishandle them, but
at least Emacs isn't introducing a bug).
- insert-file-contents overly trusts the inserted-char counts
returned by the hooks; they should be sanity checked, to avoid
the potential for calculating incorrect buffer offsets.
- concat mishandles some long strings. It checks for byte count
overflow in places where it should check for char count overflow,
and it misses some byte count overflows.
- (set-mouse-position FRAME 4294967296 4294967296) is silently
treated as if it were (set-mouse-position FRAME 0 0). There
are similar problems with set-mouse-pixel-position,
set-frame-height, set-frame-width, set-frame-size,
set-frame-position, modify-frame-parameters.
- (modify-frame-parameters FRAME ALIST) can overrun the C stack
if ALIST is long.
- (open-font FONT-ENTITY 4294967296) is silently treated as if it
were (open-font FONT-ENTITY 0).
- (destroy-fringe-bitmap BITMAP) mishandles a bitmap whose fringe
is 4294967296, by silently treating it as a fringe of 0. There
are similar issues with other fringe bitmap operations.
- (define-fringe-bitmap BITMAP BITS HEIGHT WIDTH) incorrectly
treats a HEIGHT of -4294967296 as if it were zero, and
similarly for WIDTH.
- (fringe-bitmaps-at-pos 4294967296) is silently treated as if it
were (fringe-bitmaps-at-pos 0).
- (gnutls-error-fatalp -4294967297) is silently treated as if it
were (gnutls-error-fatalp -1).
- Setting tool-bar-button-margin values to 4294967297 is silently
treated as if they were set to 1. There is a similar problem with
tool-bar-border and hscroll-step.
- (lookup-image ... :relief 4294967297) is silently treated as if
it was :relief 1.
- If auto-save-timeout is large, it wraps around with large files
and behaves as if it were negative, or zero, or small.
- (define-key KEYMAP KEY DEF) badly mishandles KEY or DEF values
whose lengths exceed 2**31. There are similar issues with
lookup-key.
- (text-char-description 4294967296) is silently treated as if it
were (text-char-description 0).
- The Lisp reader mishandles syntax errors like '(#^^[]), causing
it to read storage that is out of bounds of an array. It also
mishandles '(#^^[4294967297 ...]), causing it to treat the
large integer as if it were 1.
- The Lisp printer mishandles hash tables containing more than
2**31 elements, if print-length exceeds 2**31.
- (set-process-window-size 4294967296 4294967296) is silently
treated as (set-process-window-size 0 0).
- (set-process-datagram-address PROCESS '(4294967296 ...)) silently
treats the 4294967296 as if it were 0.
- (set-network-process-option PROCESS OPTION 4294967296) does not
diagnose the out-of-range integer, for integer options.
- (internal-describe-syntax-value '(0 . 4294967296)) silently treats
the 4294967296 as if it were 0.
- (parse-partial-sexp FROM TO 4294967296) silently treats the
4294967296 as if it were 0.
- (set-window-hscroll 4294967296) silently treats the 4294967296
as if it were 0.
- (window-line-height 4294967296) silently treats the 4294967296
as if it were 0.
- (scroll-left 4294967296) silently treats the 4294967296 as if
it were 0. There is a similar problem with scroll-right and
with hourglass-delay.
- (dump-glyph-row 4294967297 4294967297) silently treats both
instances of 4294967297 as if they were 1. There is a similar
problem with dump-tool-bar-row.
- (bitmap-spec-p '(34359738368 34359738368 "x")) incorrectly returns t.
This can cause further problems in the code that loads bitmaps.
- (x-change-window-property PROP VALUE FRAME TYPE 4294967304) silently
treats the 4294967304 as if it were 8.
* Callers to larger_vector often blindly multiply sizes by 2, which
can lead to integer overflow with large sizes. Change
larger_vector's API to make it easier check for size overflow when
growing a vector.
* Make Emacs more efficient when configured on a 32-bit host
--with-wide-int, by using ptrdiff_t instead of EMACS_INT when
ptrdiff_t suffices. This reduces the size of Emacs data
structures; for example, on one Fedora 14 test, this patch shrank
the number of pure bytes used by 11%, and shrank the executable
size (text+data) by 5.3%.
* Similarly, fix a few other places where a wide int is used even
though a narrower int suffices. For example, since the internal
buffer in Fcall_process can never exceed CALLPROC_BUFFER_SIZE_MAX
(64 KiB), its size can be stored in an int rather than in an EMACS_INT.
[-- Attachment #2: intwidth.txt.gz --]
[-- Type: application/x-gzip, Size: 161920 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 7:51 bug#9874: Fixes for several integer overflow and width issues Paul Eggert
@ 2011-10-26 8:04 ` Andreas Schwab
2011-10-26 14:09 ` Stefan Monnier
1 sibling, 0 replies; 12+ messages in thread
From: Andreas Schwab @ 2011-10-26 8:04 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9874
Paul Eggert <eggert@cs.ucla.edu> writes:
> - Conversely, Emacs assumes that uid_t, gid_t, pid_t, and file
> descriptor values fit into fixnums,
File descriptors are always small numbers.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 7:51 bug#9874: Fixes for several integer overflow and width issues Paul Eggert
2011-10-26 8:04 ` Andreas Schwab
@ 2011-10-26 14:09 ` Stefan Monnier
2011-10-26 14:26 ` Juanma Barranquero
2011-10-27 6:33 ` Paul Eggert
1 sibling, 2 replies; 12+ messages in thread
From: Stefan Monnier @ 2011-10-26 14:09 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9874
> Attached is a patch to the Emacs trunk (bzr 106194) to fix several
> issues, mostly due to integer overflow. This patch was derived by
> looking for and fixing common integer-width issues. The patch does
> not attempt to fix all these issues, just some issues that are clear
> and for which the fixes are straightforward.
Thank you very much, but we're too far in the release process for such
large patches, so it will have to wait for 24.2.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 14:09 ` Stefan Monnier
@ 2011-10-26 14:26 ` Juanma Barranquero
2011-10-26 17:02 ` David De La Harpe Golden
2011-10-26 17:16 ` Stefan Monnier
2011-10-27 6:33 ` Paul Eggert
1 sibling, 2 replies; 12+ messages in thread
From: Juanma Barranquero @ 2011-10-26 14:26 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 9874, Paul Eggert
> Thank you very much, but we're too far in the release process for such
> large patches, so it will have to wait for 24.2.
These bugs seem serious enough:
- On my Fedora 14 x86-64 host, (signal-process 4294967295 1)
crashes my entire login session, Emacs included, and leaves my
workstation in a corrupted state in which the screen
continually flashes a nonsense pattern and I cannot log in.
This bug occurs because Emacs incorrectly assumes that fixnums
fit into pid_t values, which is not true on typical 64-bit hosts.
- The following code makes Emacs dump core:
(progn
(setq code-conversion-map-vector 0)
(register-code-conversion-map 'x (make-vector 1 1)))
- (font-get-glyphs FONT-OBJECT FROM TO) goes beserk if TO - FROM
exceeds 2**31, and if you're lucky it dumps core.
- (modify-frame-parameters FRAME ALIST) can overrun the C stack
if ALIST is long.
- The Lisp reader mishandles syntax errors like '(#^^[]), causing
it to read storage that is out of bounds of an array. It also
mishandles '(#^^[4294967297 ...]), causing it to treat the
large integer as if it were 1.
- insert-file-contents overly trusts the inserted-char counts
returned by the hooks; they should be sanity checked, to avoid
the potential for calculating incorrect buffer offsets.
- concat mishandles some long strings. It checks for byte count
overflow in places where it should check for char count overflow,
and it misses some byte count overflows.
- (find-operation-coding-system 'write-region 1 2) has an
off-by-one error that causes it to access the garbage that
is one past its argument array.
* Callers to larger_vector often blindly multiply sizes by 2, which
can lead to integer overflow with large sizes. Change
larger_vector's API to make it easier check for size overflow when
growing a vector.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 14:26 ` Juanma Barranquero
@ 2011-10-26 17:02 ` David De La Harpe Golden
2011-10-26 17:16 ` Stefan Monnier
1 sibling, 0 replies; 12+ messages in thread
From: David De La Harpe Golden @ 2011-10-26 17:02 UTC (permalink / raw)
To: Juanma Barranquero; +Cc: 9874, Paul Eggert
On 26/10/11 15:26, Juanma Barranquero wrote:
>> Thank you very much, but we're too far in the release process for such
>> large patches, so it will have to wait for 24.2.
>
> These bugs seem serious enough:
>
> - On my Fedora 14 x86-64 host, (signal-process 4294967295 1)
> crashes my entire login session, Emacs included, and leaves my
> workstation in a corrupted state in which the screen
> continually flashes a nonsense pattern and I cannot log in.
> This bug occurs because Emacs incorrectly assumes that fixnums
> fit into pid_t values, which is not true on typical 64-bit hosts.
>
Hmm. The really serious/horrible part there (screen manglement and being
unable to login again) sounds like some X and/or [gkx]dm bug well
outside emacs' responsibility. Given the current state of the emacs
code, it should "just" kill (hup) all the processes you can kill*. It
shouldn't be any worse than if you just opened up an xterm and typed
"kill -HUP -1".
Yes, that probably includes suddenly ending your entire desktop session,
but unless you're running as a privileged user that should "just" leave
you staring in annoyance at the [gkx]dm login dialog, there shouldn't be
any lingering corruption, just like there shouldn't be any lingering
corruption with a "kill -HUP -1".
Emacs bug that 4294967295 silently/surprisingly becomes -1 in
signal-process? Well, probably. Emacs bug that your computer's state
somehow gets corrupted to the point you can't log in again? No.
* pid_t 32-bit, 4294967295 => 32-bit -1, and kill(-1,...) signals
everything you're allowed signal.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 14:26 ` Juanma Barranquero
2011-10-26 17:02 ` David De La Harpe Golden
@ 2011-10-26 17:16 ` Stefan Monnier
2011-10-26 19:22 ` Andreas Schwab
1 sibling, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2011-10-26 17:16 UTC (permalink / raw)
To: Juanma Barranquero; +Cc: 9874, Paul Eggert
>> Thank you very much, but we're too far in the release process for such
>> large patches, so it will have to wait for 24.2.
> These bugs seem serious enough:
I don't claim they're not serious: just too hypothetical to justify
such a large patch at such a late stage,
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 17:16 ` Stefan Monnier
@ 2011-10-26 19:22 ` Andreas Schwab
0 siblings, 0 replies; 12+ messages in thread
From: Andreas Schwab @ 2011-10-26 19:22 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 9874, Juanma Barranquero, Paul Eggert
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Thank you very much, but we're too far in the release process for such
>>> large patches, so it will have to wait for 24.2.
>> These bugs seem serious enough:
>
> I don't claim they're not serious: just too hypothetical to justify
> such a large patch at such a late stage,
Especially the EMACS_INT -> ptrdiff_t change does not fix any bug.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-26 14:09 ` Stefan Monnier
2011-10-26 14:26 ` Juanma Barranquero
@ 2011-10-27 6:33 ` Paul Eggert
2011-10-27 8:28 ` Andreas Schwab
2012-04-09 23:38 ` Paul Eggert
1 sibling, 2 replies; 12+ messages in thread
From: Paul Eggert @ 2011-10-27 6:33 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 9874
On 10/26/11 07:09, Stefan Monnier wrote:
> Thank you very much
You're welcome.
> we're too far in the release process for such large patches
OK, I'll hold off until after the release.
To follow up on two of the other comments:
* The problem with (signal-process 4294967295 1) also occurred
with (signal-process -1 1), so it's true that that spectacular
failure wasn't all Emacs's fault. (I have since upgraded to
Fedora 15 and have been too chicken to try out the test case
in my new environment....)
* File descriptors are small ints on platforms I regularly use,
but POSIX doesn't guarantee this assumption. Removing the assumption
added two lines of code. It's not a big deal either way.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-27 6:33 ` Paul Eggert
@ 2011-10-27 8:28 ` Andreas Schwab
2011-10-27 15:14 ` Paul Eggert
2012-04-09 23:38 ` Paul Eggert
1 sibling, 1 reply; 12+ messages in thread
From: Andreas Schwab @ 2011-10-27 8:28 UTC (permalink / raw)
To: Paul Eggert; +Cc: 9874
Paul Eggert <eggert@cs.ucla.edu> writes:
> * File descriptors are small ints on platforms I regularly use,
> but POSIX doesn't guarantee this assumption.
Every function returning a new file descriptor returns the smallest
available one, so if you want to get a very big one you have to have
odles of open files already (unless you force a particular descriptor
with dup2, which Emacs doesn't).
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-27 8:28 ` Andreas Schwab
@ 2011-10-27 15:14 ` Paul Eggert
0 siblings, 0 replies; 12+ messages in thread
From: Paul Eggert @ 2011-10-27 15:14 UTC (permalink / raw)
To: Andreas Schwab; +Cc: 9874
On 10/27/11 01:28, Andreas Schwab wrote:
> if you want to get a very big one you have to have
> odles of open files already
Yes, that's the scenario. The invoker of Emacs could
open oodles of file descriptors, causing Emacs to
obtain a high file-descriptor number whenever
it opens a file, creates a pipe, etc.
Admittedly unlikely, but easy to defend against.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2011-10-27 6:33 ` Paul Eggert
2011-10-27 8:28 ` Andreas Schwab
@ 2012-04-09 23:38 ` Paul Eggert
2012-05-25 20:50 ` Paul Eggert
1 sibling, 1 reply; 12+ messages in thread
From: Paul Eggert @ 2012-04-09 23:38 UTC (permalink / raw)
To: 9874
[-- Attachment #1: Type: text/plain, Size: 377 bytes --]
Since we've now created an emacs-24 branch it seems like a good time
to install this pending patch now. I'm attaching an updated copy.
It's the same as before, but updated in the light of the Emacs
changes since the patch was created. It seems large enough
(about 20,000 lines, 653 kB) that I thought it wise to compress it.
The patch is relative to bzr 107822 in the trunk.
[-- Attachment #2: intsmall-patch.txt.gz --]
[-- Type: application/x-gzip, Size: 162641 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#9874: Fixes for several integer overflow and width issues
2012-04-09 23:38 ` Paul Eggert
@ 2012-05-25 20:50 ` Paul Eggert
0 siblings, 0 replies; 12+ messages in thread
From: Paul Eggert @ 2012-05-25 20:50 UTC (permalink / raw)
To: 9874-done; +Cc: Dmitry Antipov
Stefan OKed this patch so I installed this into the trunk
as bzr 108370 and am marking the bug as done. The idea
is to give Dmitry a chance to rework his proposal for
immediate strings
<http://lists.gnu.org/archive/html/emacs-devel/2012-05/msg00435.html>
in the light of the usually-smaller data structures when
configured --with-wide-int on 32-bit hosts.
This patch affects w32fns.c and w32menu.c in what I
hope are trivial and safe ways, so I'm CC'ing this to
Eli to give him a heads-up too.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-05-25 20:50 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-26 7:51 bug#9874: Fixes for several integer overflow and width issues Paul Eggert
2011-10-26 8:04 ` Andreas Schwab
2011-10-26 14:09 ` Stefan Monnier
2011-10-26 14:26 ` Juanma Barranquero
2011-10-26 17:02 ` David De La Harpe Golden
2011-10-26 17:16 ` Stefan Monnier
2011-10-26 19:22 ` Andreas Schwab
2011-10-27 6:33 ` Paul Eggert
2011-10-27 8:28 ` Andreas Schwab
2011-10-27 15:14 ` Paul Eggert
2012-04-09 23:38 ` Paul Eggert
2012-05-25 20:50 ` Paul Eggert
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.