From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Jan =?UTF-8?Q?Dj=C3=A4rv?= Newsgroups: gmane.emacs.bugs Subject: bug#9196: integer and memory overflow issues (e.g., cut-and-paste crashes Emacs) Date: Fri, 29 Jul 2011 18:49:20 +0200 Message-ID: <4E32E490.3050002@swipnet.se> References: <4E3256E9.3020208@cs.ucla.edu> <4E3284EB.1010308@swipnet.se> <4E32DE0E.5050208@cs.ucla.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Trace: dough.gmane.org 1311958208 1600 80.91.229.12 (29 Jul 2011 16:50:08 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 29 Jul 2011 16:50:08 +0000 (UTC) Cc: 9196@debbugs.gnu.org To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jul 29 18:50:04 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1QmqGA-000647-Ok for geb-bug-gnu-emacs@m.gmane.org; Fri, 29 Jul 2011 18:50:02 +0200 Original-Received: from localhost ([::1]:37949 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmqGA-0004Pp-3g for geb-bug-gnu-emacs@m.gmane.org; Fri, 29 Jul 2011 12:50:02 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:36497) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmqG7-0004OG-5a for bug-gnu-emacs@gnu.org; Fri, 29 Jul 2011 12:50:00 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QmqG5-0008Lb-LA for bug-gnu-emacs@gnu.org; Fri, 29 Jul 2011 12:49:59 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:50743) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmqG5-0008LP-EA for bug-gnu-emacs@gnu.org; Fri, 29 Jul 2011 12:49:57 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1QmqGA-0004fa-GT; Fri, 29 Jul 2011 12:50:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Jan =?UTF-8?Q?Dj=C3=A4rv?= Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 29 Jul 2011 16:50:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 9196 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 9196-submit@debbugs.gnu.org id=B9196.131195818417923 (code B ref 9196); Fri, 29 Jul 2011 16:50:02 +0000 Original-Received: (at 9196) by debbugs.gnu.org; 29 Jul 2011 16:49:44 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QmqFr-0004f1-GH for submit@debbugs.gnu.org; Fri, 29 Jul 2011 12:49:43 -0400 Original-Received: from smtprelay-b11.telenor.se ([62.127.194.20]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QmqFp-0004et-Cm for 9196@debbugs.gnu.org; Fri, 29 Jul 2011 12:49:42 -0400 Original-Received: from ipb4.telenor.se (ipb4.telenor.se [195.54.127.167]) by smtprelay-b11.telenor.se (Postfix) with ESMTP id 58631C139 for <9196@debbugs.gnu.org>; Fri, 29 Jul 2011 18:49:34 +0200 (CEST) X-SENDER-IP: [85.225.45.26] X-LISTENER: [smtp.bredband.net] X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AoZ3AHTkMk5V4S0aPGdsb2JhbAA0AQEEASkPAQUiGAoCAQUMDBgCAgUiCwICCQMCAQIBAh4NCxsFAg4BDgEBhE+ER55NCwEBAQEeGQ0liHwCrhWRBYErhAaBEASjJjk X-IronPort-AV: E=Sophos;i="4.67,288,1309730400"; d="scan'208";a="1750926387" Original-Received: from c-1a2de155.25-1-64736c10.cust.bredbandsbolaget.se (HELO coolsville.localdomain) ([85.225.45.26]) by ipb4.telenor.se with ESMTP; 29 Jul 2011 18:49:28 +0200 Original-Received: from anon-94-147.ipredate.net (anon-94-147.ipredate.net [93.182.147.94]) by coolsville.localdomain (Postfix) with ESMTPSA id 838357FA059; Fri, 29 Jul 2011 18:49:26 +0200 (CEST) User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:5.0) Gecko/20110624 Thunderbird/5.0 In-Reply-To: <4E32DE0E.5050208@cs.ucla.edu> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Fri, 29 Jul 2011 12:50:02 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 1) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:49699 Archived-At: Paul Eggert skrev 2011-07-29 18:21: > First, thanks for taking a look at the patch. And in response to your > comments ... > > On 07/29/11 03:01, Jan Dj=C3=A4rv wrote: > >> a crash would be fine by me. Actually better than memory_full, >> because the core is much more useful. > > We can easily arrange to crash, by replacing "memory_full (SIZE_MAX)" > with "abort ()". I can do that for places where that's preferred. It > sounds like xgselect.c is one of those places, so I'll do that; please > let me know of any other places. The problem with abort is that gcc may optimize them to look like they=20 occurred elsewhere. > I systematically looked for all places where memory is being > allocated, and where size calculations might overflow during this, and > fixed them all. It's better to have a systematic policy where all > such size calculations are checked. Trying to decide heuristically > which size overflows don't need checking because they can "never > happen" would lead to further sources of maintenance error, and anyway > the idea that some size overflows can "never happen" is uncomfortably > close to the apocryphal story that "640K of memory should be enough > for anybody". It is also a maintenance burden to keep all checks. More code that can g= o=20 wrong, but it really add nothing. The difference with 640k is enough and= 2=20 billion scroll bars is huge. If you create a scroll bar every tenth seco= nd,=20 it will take approx 6.8 years to reach 2 billion. Why is it important to= =20 check for that? I do think it is important to reflect on what the code d= oes=20 and how limits are reached before adding more code that increases CPU and= =20 memory usage for no real benefit. >> In xgselect.c: >> >> + int gfds_size_max =3D >> + min (INT_MAX, min (PTRDIFF_MAX, SIZE_MAX) / sizeof *gfds); >> >> Here a compile time constant is recalculated inside a loop. > > Since it's a constant, the runtime cost of calculating it is zero, so > there's no efficiency gain by moving it. I think you are assuming compiler optimizations here. They might be true= for=20 gcc, but not for other compilers. A define more clearly states that it i= s a=20 constant than a computed variable. > Do you think it would be > clearer to hoist it out of the loop? Yes, and by declaring it const. But even better, a define. > If so, we can easily do that; > but there is something to be said for having the definition of the > constant near its use. > A define can be put anywhere. >> The xgselect.c is also overengineering IMHO. The number checked >> represents the number of file descriptor sources Glib is checking. >> I can understand checking sizes for strings that come from external >> sources, but only code adds file descriptor sources. If some bug >> causes the addition of 2 billion sources > > 2 billion sources is not always the limit. On typical 32-bit hosts, > the current code stops working at 500 million sources, or 250 million > if one is worried about pointer subtraction. And if future glib > versions change the associated struct, that 250-million limit could go > down further. In cases like these, it's helpful to check the limit > even if the check can "never fail". 250 million is way more than any OS can handle. What is the point of hav= ing=20 Emacs check stuff that the OS simply can't handle anyway? Please come up= with=20 a real scenario when this may actually fail before adding checks. You ar= en't=20 the one that has to take the time to update these checks when the code=20 changes. You are putting maintenance burden on others without any real b= enefit. Jan D.