This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Prevent overflow in rl_set_screen_size
- From: Kevin Buettner <kevinb at redhat dot com>
- To: gdb-patches at sourceware dot org
- Cc: Saagar Jha <saagar at saagarjha dot com>
- Date: Thu, 14 Feb 2019 18:52:54 -0700
- Subject: Re: [PATCH] Prevent overflow in rl_set_screen_size
- References: <A2D1267E-9CB0-49C2-B3AC-4F880A7951E2@saagarjha.com>
On Fri, 26 Oct 2018 21:56:50 -0700
Saagar Jha <saagar@saagarjha.com> wrote:
> 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.
This seems like a reasonable approach to me. (I couldn't think of a
better way to do it.)
> gdb/ChangeLog:
> 2018-05-22 Saagar Jha <saagar@saagarjha.com>
>
> * utils.c: Reduce "infinite" rows and columns before calling
> rl_set_screen_size.
When you check this in, make sure that you adjust the date. Also,
the ChangeLog comment should include the name of the affected function.
So it'll look something like this:
* utils.c (set_screen_size): Reduce "infinite" rows and columns
before calling rl_set_screen_size.
The actual content of the patch looks good to me.
Thanks,
Kevin
> ---
> gdb/utils.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> 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 = lines_per_page;
> int cols = chars_per_line;
>
> + // 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
> if (rows <= 0)
> - rows = INT_MAX;
> + rows = INT_MAX >> (sizeof(int) * 8 / 2);
>
> if (cols <= 0)
> - cols = INT_MAX;
> + cols = INT_MAX >> (sizeof(int) * 8 / 2);
>
> /* Update Readline's idea of the terminal size. */
> rl_set_screen_size (rows, cols);
> --
> 2.19.1
>
>