[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: oops? read/write vs type of length parameter
From: |
Paul Eggert |
Subject: |
Re: oops? read/write vs type of length parameter |
Date: |
Tue, 12 Apr 2011 22:14:48 -0700 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8 |
On 04/12/2011 08:53 AM, Paul Eggert wrote:
> I can do it, in the next day or two.
OK, I did it, by auditing all uses of emacs_read and emacs_write to
find and fix all negative buffer sizes. However, instead of negative
sizes I found overflows and some other issues, described below; and I
therefore now agree with Eli that emacs_write shouldn't use the same
signature as POSIX 'write'. However, I also agree with Jim that
emacs_write's API need not artificially limit buffer sizes to SSIZE_MAX.
Here's the key idea of the attached patch. Since
"N = emacs_write (FD, BUF, SIZE)" returns N if successful, and some
value less than N if it fails, the caller can easily determine whether
it succeeded by testing N == SIZE. So on failure there's no need for
emacs_write to return a special value like -1.
With this in mind, emacs_write can simply return the number of bytes
written, which will always be a size_t value. Jim, this idea is
already used by gnulib's full_write function, so there's good
precedent for departing from the POSIX signature here. And Eli, this
avoids the problem where the size is so large that a signed return
value would overflow and become negative.
Here are some other bugs in Emacs that I found as part of this audit:
* Several places in sound.c attempt to stuff a size into an 'int',
which doesn't work. This problem is independent of the above, and
its fix is independent of the above too, so it's split into a
separate patch in the attached bundle.
* send_process attempts to stuff a size into an 'int' as well. This
can be fixed by changing one of its local variables from int to size_t.
* emacs_gnutls_write has the same issues as emacs_write, and should
be fixed the same way.
Attached is a bzr bundle of two proposed patches to fix the problem as
described above. Comments are welcome.
patch-emacs-write.txt
Description: Text document
- Re: oops? read/write vs type of length parameter, (continued)
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/11
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/11
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/11
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/12
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/12
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/12
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/12
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/12
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/12
- Re: oops? read/write vs type of length parameter, Juanma Barranquero, 2011/04/12
- Re: oops? read/write vs type of length parameter,
Paul Eggert <=
- Re: oops? read/write vs type of length parameter, Jim Meyering, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/13
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13
- Re: oops? read/write vs type of length parameter, PJ Weisberg, 2011/04/13
- Re: oops? read/write vs type of length parameter, Eli Zaretskii, 2011/04/14
- Re: oops? read/write vs type of length parameter, Paul Eggert, 2011/04/13