From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: oops? read/write vs type of length parameter Date: Tue, 12 Apr 2011 01:19:10 -0700 Organization: UCLA Computer Science Department Message-ID: <4DA40AFE.8050406@cs.ucla.edu> References: <87wrj1jhfc.fsf@rho.meyering.net> <87hba5yq0p.fsf@uwakimon.sk.tsukuba.ac.jp> <834o64sxd7.fsf@gnu.org> <4DA3A7F8.1020503@cs.ucla.edu> <83k4f0qijz.fsf@gnu.org> <4DA3DDCD.10700@cs.ucla.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1302596384 7772 80.91.229.12 (12 Apr 2011 08:19:44 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 12 Apr 2011 08:19:44 +0000 (UTC) Cc: Jim Meyering , emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Apr 12 10:19:40 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from [140.186.70.17] (helo=lists.gnu.org) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Q9Yp2-0004k5-0f for ged-emacs-devel@m.gmane.org; Tue, 12 Apr 2011 10:19:40 +0200 Original-Received: from localhost ([::1]:36651 helo=lists2.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9Yp1-0001RM-HN for ged-emacs-devel@m.gmane.org; Tue, 12 Apr 2011 04:19:39 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:44972) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9Yov-0001QN-71 for emacs-devel@gnu.org; Tue, 12 Apr 2011 04:19:38 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9Yoo-0007yc-7p for emacs-devel@gnu.org; Tue, 12 Apr 2011 04:19:33 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:51929) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9Yom-0007yD-L0; Tue, 12 Apr 2011 04:19:24 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 8ED5A39E80DB; Tue, 12 Apr 2011 01:19:23 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U9y3j8W5xe9i; Tue, 12 Apr 2011 01:19:21 -0700 (PDT) Original-Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 812BB39E8083; Tue, 12 Apr 2011 01:19:21 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8 In-Reply-To: <4DA3DDCD.10700@cs.ucla.edu> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 131.179.128.62 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org X-Broken-Reverse-DNS: no host name found for IP address 140.186.70.17 Xref: news.gmane.org gmane.emacs.devel:138431 Archived-At: On looking over that code again, a couple of issues sprang out. First, emacs_read should act like emacs_write with respect to sizes, but the code didn't do that. Second, no caller should ever pass a negative size value to either function, and callers should not rely on negative sizes causing emacs_read and emacs_write to do nothing. I added a runtime check for this, which I don't think will ever fail, but I've been surprised in the past. With that check in place we might as well use size_t for the size, with the goal of removing the runtime checks once we have carefully checked that they aren't needed. Here's the patch I installed for that. * sysdep.c (emacs_read, emacs_write): Check for negative sizes since callers should never pass a negative size. Change the signature to match that of plain 'read' and 'write'; see . * lisp.h: Update prototypes of emacs_write and emacs_read. === modified file 'src/lisp.h' --- src/lisp.h 2011-04-10 20:43:08 +0000 +++ src/lisp.h 2011-04-12 08:05:04 +0000 @@ -3346,8 +3346,8 @@ extern void seed_random (long); extern int emacs_open (const char *, int, int); extern int emacs_close (int); -extern ssize_t emacs_read (int, char *, ssize_t); -extern ssize_t emacs_write (int, const char *, ssize_t); +extern ssize_t emacs_read (int, char *, size_t); +extern ssize_t emacs_write (int, const char *, size_t); enum { READLINK_BUFSIZE = 1024 }; extern char *emacs_readlink (const char *, char [READLINK_BUFSIZE]); #ifndef HAVE_MEMSET === modified file 'src/sysdep.c' --- src/sysdep.c 2011-04-10 20:43:08 +0000 +++ src/sysdep.c 2011-04-12 08:05:09 +0000 @@ -1826,10 +1826,18 @@ } ssize_t -emacs_read (int fildes, char *buf, ssize_t nbyte) +emacs_read (int fildes, char *buf, size_t nbyte) { register ssize_t rtnval; + /* Defend against the possibility that a buggy caller passes a negative NBYTE + argument, which would be converted to a large unsigned size_t NBYTE. This + defense prevents callers from doing large writes, unfortunately. This + size restriction can be removed once we have carefully checked that there + are no such callers. */ + if ((ssize_t) nbyte < 0) + abort (); + while ((rtnval = read (fildes, buf, nbyte)) == -1 && (errno == EINTR)) QUIT; @@ -1837,13 +1845,17 @@ } ssize_t -emacs_write (int fildes, const char *buf, ssize_t nbyte) +emacs_write (int fildes, const char *buf, size_t nbyte) { register ssize_t rtnval, bytes_written; + /* Defend against negative NBYTE, as in emacs_read. */ + if ((ssize_t) nbyte < 0) + abort (); + bytes_written = 0; - while (nbyte > 0) + while (nbyte != 0) { rtnval = write (fildes, buf, nbyte);