From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#9196: integer and memory overflow issues (e.g., cut-and-paste crashes Emacs) Date: Fri, 29 Jul 2011 14:03:05 -0700 Organization: UCLA Computer Science Department Message-ID: <4E332009.3090909@cs.ucla.edu> References: <4E3256E9.3020208@cs.ucla.edu> <4E3284EB.1010308@swipnet.se> <4E32DE0E.5050208@cs.ucla.edu> <4E32E490.3050002@swipnet.se> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Trace: dough.gmane.org 1311973447 29750 80.91.229.12 (29 Jul 2011 21:04:07 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 29 Jul 2011 21:04:07 +0000 (UTC) Cc: 9196@debbugs.gnu.org To: Jan =?UTF-8?Q?Dj=C3=A4rv?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jul 29 23:04:02 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 1QmuDw-0001yq-GY for geb-bug-gnu-emacs@m.gmane.org; Fri, 29 Jul 2011 23:04:00 +0200 Original-Received: from localhost ([::1]:39155 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmuDv-00012u-L2 for geb-bug-gnu-emacs@m.gmane.org; Fri, 29 Jul 2011 17:03:59 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:56639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmuDs-00012c-3a for bug-gnu-emacs@gnu.org; Fri, 29 Jul 2011 17:03:57 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QmuDr-0006AI-0K for bug-gnu-emacs@gnu.org; Fri, 29 Jul 2011 17:03:56 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:38683) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmuDq-0006AE-Tg for bug-gnu-emacs@gnu.org; Fri, 29 Jul 2011 17:03:54 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1QmuDx-0001rZ-Kz; Fri, 29 Jul 2011 17:04:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert 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 21:04:01 +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.13119734017113 (code B ref 9196); Fri, 29 Jul 2011 21:04:01 +0000 Original-Received: (at 9196) by debbugs.gnu.org; 29 Jul 2011 21:03:21 +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 1QmuDJ-0001qf-5u for submit@debbugs.gnu.org; Fri, 29 Jul 2011 17:03:21 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QmuDG-0001qW-Pe for 9196@debbugs.gnu.org; Fri, 29 Jul 2011 17:03:20 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 16AFD39E80D2; Fri, 29 Jul 2011 14:03:11 -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 Hwbjwi+h9Pl2; Fri, 29 Jul 2011 14:03:10 -0700 (PDT) Original-Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 51DFF39E80CF; Fri, 29 Jul 2011 14:03:10 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.18) Gecko/20110621 Fedora/3.1.11-1.fc14 Thunderbird/3.1.11 In-Reply-To: <4E32E490.3050002@swipnet.se> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Fri, 29 Jul 2011 17:04:01 -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:49701 Archived-At: On 07/29/11 09:49, Jan Dj=C3=A4rv wrote: >>> + 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. >=20 > I think you are assuming compiler optimizations here. They might be > true for gcc, but not for other compilers. A define more clearly > states that it is a constant than a computed variable. Sorry, I don't follow this. A 'define' wouldn't decrease the runtime cost, as a non-optimizing compiler would evaluate the expansion of the 'define' at runtime. Anyway, to address this issue I'll change it to an enum, which is guaranteed to be calculated at compile-time and clearly marks it as a constant. And I'll hoist it out of the function entirely. Something like this should do the trick: static GPollFD *gfds; static int gfds_size; enum { GFDS_SIZE_MAX =3D min (INT_MAX, min (PTRDIFF_MAX, SIZE_MAX) / sizeof *gfds) }; > The problem with abort is that gcc may optimize them to look > like they occurred elsewhere. abort () is the standard way to trap within Emacs and other GNU programs. If there are problems with debugging calls to abort (), then we should address those problems, but surely that's a separate issue. Anyway, it's much easier to debug an abort () than a memory corruption. > It is also a maintenance burden to keep all checks.... > Please come up with a real scenario when this may actually fail > before adding checks. The scenario is when Emacs has more than about 250 million event sources. Admittedly this is quite unlikely now, but it may happen in the future, and the fix is easy now. Why not make sure Emacs is robust? We're talking about only two lines of code here: if (GFDS_SIZE_MAX / 2 < n_gfds) memory_full (SIZE_MAX); which take only 2 instructions in the typical case. Surely this is not too high a price to pay for reliability. If it's the code clutter that's the major objection here, we can use an inline function to shorten it to just one line, like this: check_size (n_gfds <=3D GFDS_SIZE_MAX / 2); This would have the same run-time cost (2 instructions), but would be easier to read. How about that idea? > You aren't the one that has to take the time > to update these checks when the code changes. If the code changes in such a way that these checks need to change, then the presence of the checks is a good thing. The checks will help remind programmers of the limits involved, and will help them avoid memory corruption in the future. Currently these issues are only in programmers' heads and are too often forgotten, and this can easily lead to bugs. Also, it's not really true that I won't be the one that has to take the time. I have been taking the time to maintain and improve these checks for months now. I've found several serious bugs in the process, some of which allow remote exploits. I expect to find more bugs, and I'll be happy to help in any future problems that crop up in this area. The goal is to have an Emacs implementation that is robust= , rather than one that crashes when given input that was thought "couldn't happen".