GNU bug report logs - #11288
avoid buffer overrun in display code

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11288 in the body.
You can then email your comments to 11288 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: bug-emacs <at> gnu.org
Cc: Jakub Jelinek <jakub <at> redhat.com>,
	Richard Guenther <richard.guenther <at> gmail.com>
Subject: avoid buffer overrun in display code
Date: Fri, 20 Apr 2012 13:42:05 +0200

This bug leads to a seemingly unterminated loop in swap_glyph_pointers,
when compiled with gcc-4.8.0 (from April 19 or newer).
At first I thought it was a code-gen bug and reported it as
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53053.  But then Richard
Guenther guessed at the cause and Jakub Jelinek confirmed that the
seemingly-infinite-loop was in fact just part of the undefined behavior
we may now expect from buggy code.

2012-04-20  Jim Meyering  <meyering <at> redhat.com>

	* dispextern.h (glyph_row.used): Increase size by 1, to avoid buffer
        overrun in swap_glyph_pointers, which reads and writes used[LAST_AREA].
        Reported as a gcc bug http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53053
        where Jakub Jelinek spotted the root cause.

=== modified file 'src/dispextern.h'
--- src/dispextern.h	2012-03-26 05:43:05 +0000
+++ src/dispextern.h	2012-04-20 11:14:29 +0000
@@ -748,7 +748,7 @@
   struct glyph *glyphs[1 + LAST_AREA];

   /* Number of glyphs actually filled in areas.  */
-  short used[LAST_AREA];
+  short used[1 + LAST_AREA];

   /* Window-relative x and y-position of the top-left corner of this
      row.  If y < 0, this means that eabs (y) pixels of the row are

Message #10 received at 11288-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Meyering <jim <at> meyering.net>
Cc: jakub <at> redhat.com, richard.guenther <at> gmail.com, 11288-done <at> debbugs.gnu.org
Subject: Re: bug#11288: avoid buffer overrun in display code
Date: Fri, 20 Apr 2012 17:10:53 +0300

> From: Jim Meyering <jim <at> meyering.net>
> Date: Fri, 20 Apr 2012 13:42:05 +0200
> Cc: Jakub Jelinek <jakub <at> redhat.com>,
> 	Richard Guenther <richard.guenther <at> gmail.com>
> 
> This bug leads to a seemingly unterminated loop in swap_glyph_pointers,
> when compiled with gcc-4.8.0 (from April 19 or newer).
> At first I thought it was a code-gen bug and reported it as
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53053.  But then Richard
> Guenther guessed at the cause and Jakub Jelinek confirmed that the
> seemingly-infinite-loop was in fact just part of the undefined behavior
> we may now expect from buggy code.
> 
> 2012-04-20  Jim Meyering  <meyering <at> redhat.com>
> 
> 	* dispextern.h (glyph_row.used): Increase size by 1, to avoid buffer
>         overrun in swap_glyph_pointers, which reads and writes used[LAST_AREA].
>         Reported as a gcc bug http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53053
>         where Jakub Jelinek spotted the root cause.
> 
> === modified file 'src/dispextern.h'
> --- src/dispextern.h	2012-03-26 05:43:05 +0000
> +++ src/dispextern.h	2012-04-20 11:14:29 +0000
> @@ -748,7 +748,7 @@
>    struct glyph *glyphs[1 + LAST_AREA];
> 
>    /* Number of glyphs actually filled in areas.  */
> -  short used[LAST_AREA];
> +  short used[1 + LAST_AREA];
> 
>    /* Window-relative x and y-position of the top-left corner of this
>       row.  If y < 0, this means that eabs (y) pixels of the row are

Thanks, I fixed it somewhat differently, see below.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-04-20 06:39:29 +0000
+++ src/ChangeLog	2012-04-20 14:07:46 +0000
@@ -1,3 +1,8 @@
+2012-04-20  Eli Zaretskii  <eliz <at> gnu.org>
+
+	* dispnew.c (swap_glyph_pointers, copy_row_except_pointers): Don't
+	overrun array limits of glyph row's used[] array.  (Bug#11288)
+
 2012-04-20  Chong Yidong  <cyd <at> gnu.org>
 
 	* process.c (wait_reading_process_output): If EIO occurs on a pty,

=== modified file 'src/dispnew.c'
--- src/dispnew.c	2012-03-20 08:52:11 +0000
+++ src/dispnew.c	2012-04-20 14:04:13 +0000
@@ -1085,12 +1085,16 @@ swap_glyph_pointers (struct glyph_row *a
   for (i = 0; i < LAST_AREA + 1; ++i)
     {
       struct glyph *temp = a->glyphs[i];
-      short used_tem = a->used[i];
 
       a->glyphs[i] = b->glyphs[i];
       b->glyphs[i] = temp;
-      a->used[i] = b->used[i];
-      b->used[i] = used_tem;
+      if (i < LAST_AREA)
+	{
+	  short used_tem = a->used[i];
+
+	  a->used[i] = b->used[i];
+	  b->used[i] = used_tem;
+	}
     }
   a->hash = b->hash;
   b->hash = hash_tem;
@@ -1105,7 +1109,7 @@ static inline void
 copy_row_except_pointers (struct glyph_row *to, struct glyph_row *from)
 {
   struct glyph *pointers[1 + LAST_AREA];
-  short used[1 + LAST_AREA];
+  short used[LAST_AREA];
   unsigned hashval;
 
   /* Save glyph pointers of TO.  */

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.