[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: 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.

Attachment: patch-emacs-write.txt
Description: Text document

reply via email to

[Prev in Thread] Current Thread [Next in Thread]