From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 105614 invoked by alias); 15 Feb 2019 10:52:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 105605 invoked by uid 89); 15 Feb 2019 10:52:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-26.6 required=5.0 tests=BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=here=e2, H*c:MHil, H*c:HpplH, Hopefully?= X-HELO: mail-pf1-f193.google.com Received: from mail-pf1-f193.google.com (HELO mail-pf1-f193.google.com) (209.85.210.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 15 Feb 2019 10:52:14 +0000 Received: by mail-pf1-f193.google.com with SMTP id g6so4654624pfh.13 for ; Fri, 15 Feb 2019 02:52:14 -0800 (PST) Return-Path: Received: from resnet-11-41.resnet.ucsb.edu (ResNet-11-41.resnet.ucsb.edu. [169.231.11.41]) by smtp.gmail.com with ESMTPSA id m7sm11675317pfj.162.2019.02.15.02.52.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 15 Feb 2019 02:52:12 -0800 (PST) From: Saagar Jha Message-Id: <2077946E-DA12-459A-911E-4F271DD39913@saagarjha.com> Content-Type: multipart/mixed; boundary="Apple-Mail=_554CDC15-A84A-4E16-8F4B-3BD4EB4F1432" Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.2\)) Subject: Re: [PATCH] Prevent overflow in rl_set_screen_size Date: Fri, 15 Feb 2019 10:52:00 -0000 In-Reply-To: <9e42397a-e3a5-0296-d239-70f4c7c0d215@redhat.com> Cc: Kevin Buettner , gdb-patches@sourceware.org To: Pedro Alves References: <20190214185254.15369a0a@f29-4.lan> <9e42397a-e3a5-0296-d239-70f4c7c0d215@redhat.com> X-SW-Source: 2019-02/txt/msg00253.txt.bz2 --Apple-Mail=_554CDC15-A84A-4E16-8F4B-3BD4EB4F1432 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Content-length: 427 Here=E2=80=99s a new patch. I can try to see what readline is willing to do= regarding documenting this limitation, but that might take a while because= I=E2=80=99d have to figure out where to send contributions and the process= to getting them merged in (I *think* I=E2=80=99m supposed to send an email= to bug-readline@gnu.org?) Hopefully this patch is useful on its own until= readline is fixed. Regards, Saagar Jha --Apple-Mail=_554CDC15-A84A-4E16-8F4B-3BD4EB4F1432 Content-Disposition: attachment; filename=Prevent-overflow-in-rl_set_screen_size.patch Content-Type: application/octet-stream; x-unix-mode=0644; name="Prevent-overflow-in-rl_set_screen_size.patch" Content-Transfer-Encoding: quoted-printable Content-length: 1786 >From 7955608ce06dbd8944292b2318b90863d3a82ae1 Mon Sep 17 00:00:00 2001=0A= From: Saagar Jha =0A= Date: Tue, 22 May 2018 04:08:40 -0700=0A= Subject: [PATCH] Prevent overflow in rl_set_screen_size=0A= =0A= GDB calls rl_set_screen_size in readline with the current screen size,=0A= measured in rows and columns. To represent "infinite" sizes, GDB passes=0A= in INT_MAX; however, since rl_set_screen_size internally multiplies the=0A= number of rows and columns, this causes a signed integer overflow. To=0A= prevent this we can instead pass in the approximate square root of=0A= INT_MAX (which is still reasonably large), so that even when the number=0A= of rows and columns is "infinite" we don't overflow.=0A= =0A= gdb/ChangeLog:=0A= 2019-02-15 Saagar Jha =0A= =0A= * utils.c (set_screen_size): Reduce "infinite" rows and columns=0A= before calling rl_set_screen_size.=0A= ---=0A= gdb/utils.c | 6 ++++--=0A= 1 file changed, 4 insertions(+), 2 deletions(-)=0A= =0A= diff --git a/gdb/utils.c b/gdb/utils.c=0A= index 6fb5736abb..abca7337ab 100644=0A= --- a/gdb/utils.c=0A= +++ b/gdb/utils.c=0A= @@ -1376,11 +1376,13 @@ set_screen_size (void)=0A= int rows =3D lines_per_page;=0A= int cols =3D chars_per_line;=0A= =20=0A= + /* Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don't= =0A= + overflow in rl_set_screen_size, which multiplies rows and columns. */= =0A= if (rows <=3D 0)=0A= - rows =3D INT_MAX;=0A= + rows =3D INT_MAX >> (sizeof( int ) * 8 / 2);=0A= =20=0A= if (cols <=3D 0)=0A= - cols =3D INT_MAX;=0A= + cols =3D INT_MAX >> (sizeof( int ) * 8 / 2);=0A= =20=0A= /* Update Readline's idea of the terminal size. */=0A= rl_set_screen_size (rows, cols);=0A= --=20=0A= 2.20.1=0A= =0A= --Apple-Mail=_554CDC15-A84A-4E16-8F4B-3BD4EB4F1432 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Content-length: 2420 > On Feb 15, 2019, at 01:39, Pedro Alves wrote: >=20 > On 02/15/2019 01:52 AM, Kevin Buettner wrote: >> On Fri, 26 Oct 2018 21:56:50 -0700 >> Saagar Jha wrote: >>=20 >>> GDB calls rl_set_screen_size in readline with the current screen size, >>> measured in rows and columns. To represent "infinite" sizes, GDB passes >>> in INT_MAX; however, since rl_set_screen_size internally multiplies the >>> number of rows and columns, this causes a signed integer overflow. To >>> prevent this we can instead pass in the approximate square root of >>> INT_MAX (which is still reasonably large), so that even when the number >>> of rows and columns is "infinite" we don't overflow. >>=20 >> This seems like a reasonable approach to me. (I couldn't think of a >> better way to do it.) >=20 > It might be reasonable to have this as workaround, but pedantically, > shouldn't this be fixed in readline? The function's > documentation doesn't say anything about upper limits: >=20 > "Function: void rl_set_screen_size (int rows, int cols) > Set Readline's idea of the terminal size to rows rows and cols column= s. > If either rows or columns is less than or equal to 0, Readline's idea > of that terminal dimension is unchanged." >=20 > so if the function takes int parameters without specifying an upper bound= , it > seems like a readline bug to me to not consider large numbers. >=20 > A couple comments on formatting below. >=20 >>> diff --git a/gdb/utils.c b/gdb/utils.c >>> index 8d4a744e71..56257c35cf 100644 >>> --- a/gdb/utils.c >>> +++ b/gdb/utils.c >>> @@ -1377,11 +1377,13 @@ set_screen_size (void) >>> int rows =3D lines_per_page; >>> int cols =3D chars_per_line; >>>=20 >>> + // Use approximately sqrt(INT_MAX) instead of INT_MAX so that we don= 't >>> + // overflow in rl_set_screen_size, which multiplies rows and columns >=20 > Please use /**/ for comments, and end the sentence with a period. >=20 >>> if (rows <=3D 0) >>> - rows =3D INT_MAX; >>> + rows =3D INT_MAX >> (sizeof(int) * 8 / 2); >=20 > Space before parens in "sizeof(int)". >=20 >>>=20 >>> if (cols <=3D 0) >>> - cols =3D INT_MAX; >>> + cols =3D INT_MAX >> (sizeof(int) * 8 / 2); >=20 > Ditto. >=20 >>>=20 >>> /* Update Readline's idea of the terminal size. */ >>> rl_set_screen_size (rows, cols); >>> --=20 >>> 2.19.1 >>>=20 >>>=20 >=20 >=20 > Thanks, > Pedro Alves --Apple-Mail=_554CDC15-A84A-4E16-8F4B-3BD4EB4F1432--