Comment 22 for bug 10973

Message-ID: <email address hidden>
Date: Fri, 25 Mar 2005 20:13:45 -0500
From: Branden Robinson <email address hidden>
To: <email address hidden>, <email address hidden>
Subject: Re: Bug#284448: xserver-xfree86: xserver (ATI or Radeon something 7500) crashes on
 variouslaunches of programcs from within X.

--1fZJyN7nFm/tosmV
Content-Type: multipart/mixed; boundary="KC+fneiph5CALyUl"
Content-Disposition: inline

--KC+fneiph5CALyUl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

reassign 284448 xfree86
retitle 284448 xfree86: font library has very poor bounds-checking and can =
SEGV xfs and the X server
tag 284448 =3D upstream fixed-upstream patch
thanks

On Fri, Dec 17, 2004 at 12:22:25PM +0100, Thomas Winischhofer wrote:
> This looks like an Xlibs bug.

Yeah, it's one of those annoying static libraries that is linked both into
xfs and the X server.

> From the fact that "pd" is set to a legal value in the debugging=20
> output, while "buf" (after adding "pi->data_len") is "out of bounds" I=20
> would very much assume that "pi->data_len" contains garbage.
>=20
> As regards why it does this, I have no idea.
>=20
> Are these patches in the Debian SVN:
>=20
> http://freedesktop.org/cgi-bin/viewcvs.cgi/xlibs/Xfont/fc/fserve.c?r1=3D3=
=2E22&r2=3D3.22.2.1
> http://freedesktop.org/cgi-bin/viewcvs.cgi/xlibs/Xfont/fc/fserve.c?r1=3D3=
=2E23&r2=3D3.24
> http://cvsweb.xfree86.org/cvsweb/xc/lib/font/fc/fserve.c.diff?r1=3D3.26&r=
2=3D3.27

No. Fortunately all of the above predate the XFree86 1.1 relicensing.

I'm attaching a patch that should be bolted onto
debian/patches/000_stolen_from_HEAD.diff.

--=20
G. Branden Robinson | Damnit, we're all going to die;
Debian GNU/Linux | let's die doing something *useful*!
<email address hidden> | -- Hal Clement, on comments that
http://people.debian.org/~branden/ | space exploration is dangerous

--KC+fneiph5CALyUl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="fix_fserve.diff"
Content-Transfer-Encoding: quoted-printable

  3.25 +52 -2 xc/lib/font/fc/fserve.c
   603. Add font bounds checking to the X server side of the font server
        interface (Chisato Yamauchi, David Dawes).

  3.26 +18 -35 xc/lib/font/fc/fserve.c
  Combine two sets of bounds tests into one. (Chisato Yamauchi)

  3.27 +2 -2 xc/lib/font/fc/fserve.c
  Fix potential segfault.

Index: xc/lib/font/fc/fserve.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/xc/lib/font/fc/fserve.c,v
retrieving revision 3.22.2.1
retrieving revision 3.27
diff -u -r3.22.2.1 -r3.27
--- xc/lib/font/fc/fserve.c 29 Aug 2003 18:05:09 -0000 3.22.2.1
+++ xc/lib/font/fc/fserve.c 12 Jan 2004 17:19:30 -0000 3.27
@@ -24,7 +24,7 @@
 in this Software without prior written authorization from The Open Group.
=20
 */
-/* $XFree86: xc/lib/font/fc/fserve.c,v 3.22.2.1 2003/08/29 18:05:09 herrb =
Exp $ */
+/* $XFree86: xc/lib/font/fc/fserve.c,v 3.27 2004/01/12 17:19:30 tsi Exp $ =
*/
=20
 /*
  * Copyright 1990 Network Computing Devices
@@ -87,13 +87,13 @@
         (pci)->descent || \
         (pci)->characterWidth)
=20
+extern void ErrorF(const char *f, ...);
=20
 static int fs_read_glyphs ( FontPathElementPtr fpe, FSBlockDataPtr blockre=
c );
 static int fs_read_list ( FontPathElementPtr fpe, FSBlockDataPtr blockrec =
);
 static int fs_read_list_info ( FontPathElementPtr fpe,=20
           FSBlockDataPtr blockrec );
=20
-static int fs_font_type;
 extern fd_set _fs_fd_mask;
=20
 static void fs_block_handler ( pointer data, OSTimePtr wt,=20
@@ -952,6 +952,7 @@
     CharInfoPtr ci, pCI;
     char *fsci;
     fsXCharInfo fscilocal;
+ FontInfoRec *fi =3D &bfont->pfont->info;
=20
     rep =3D (fsQueryXExtents16Reply *) fs_get_reply (conn, &ret);
     if (!rep || rep->type =3D=3D FS_Error)
@@ -997,6 +998,21 @@
     {
  memcpy(&fscilocal, fsci, SIZEOF(fsXCharInfo)); /* align it */
  _fs_convert_char_info(&fscilocal, &ci->metrics);
+ /* Bounds check. */
+ if (ci->metrics.ascent > fi->maxbounds.ascent)
+ {
+ ErrorF("fserve: warning: %s %s ascent (%d) > maxascent (%d)\n",
+ fpe->name, fsd->name,
+ ci->metrics.ascent, fi->maxbounds.ascent);
+ ci->metrics.ascent =3D fi->maxbounds.ascent;
+ }
+ if (ci->metrics.descent > fi->maxbounds.descent)
+ {
+ ErrorF("fserve: warning: %s %s descent (%d) > maxdescent (%d)\n",
+ fpe->name, fsd->name,
+ ci->metrics.descent, fi->maxbounds.descent);
+ ci->metrics.descent =3D fi->maxbounds.descent;
+ }
  fsci =3D fsci + SIZEOF(fsXCharInfo);
  /* Initialize the bits field for later glyph-caching use */
  if (NONZEROMETRICS(&ci->metrics))
@@ -1022,7 +1038,6 @@
     /* build bitmap metrics, ImageRectMax style */
     if (haveInk)
     {
- FontInfoRec *fi =3D &bfont->pfont->info;
  CharInfoPtr ii;
=20
  ci =3D fsfont->encoding;
@@ -1042,6 +1057,23 @@
      {
   ci->metrics =3D ii->metrics;
      }
+ /* Bounds check. */
+ if (ci->metrics.ascent > fi->maxbounds.ascent)
+ {
+ ErrorF("fserve: warning: %s %s ascent (%d) "
+ "> maxascent (%d)\n",
+ fpe->name, fsd->name,
+ ci->metrics.ascent, fi->maxbounds.ascent);
+ ci->metrics.ascent =3D fi->maxbounds.ascent;
+ }
+ if (ci->metrics.descent > fi->maxbounds.descent)
+ {
+ ErrorF("fserve: warning: %s %s descent (%d) "
+ "> maxdescent (%d)\n",
+ fpe->name, fsd->name,
+ ci->metrics.descent, fi->maxbounds.descent);
+ ci->metrics.descent =3D fi->maxbounds.descent;
+ }
  }
     }
     {
@@ -1498,7 +1530,6 @@
     FSBlockDataPtr blockrec =3D NULL;
     FSBlockedFontPtr bfont;
     FSFontDataPtr fsd;
- FSFontPtr fsfont;
     fsOpenBitmapFontReq openreq;
     fsQueryXInfoReq inforeq;
     fsQueryXExtents16Req extreq;
@@ -1522,7 +1553,6 @@
=20
  font =3D *ppfont;
  fsd =3D (FSFontDataPtr)font->fpePrivate;
- fsfont =3D (FSFontPtr)font->fontPrivate;
  /* This is an attempt to reopen a font. Did the font have a
     NAME property? */
  if ((nameatom =3D MakeAtom("FONT", 4, 0)) !=3D None)
@@ -1550,7 +1580,6 @@
      return AllocError;
 =09
  fsd =3D (FSFontDataPtr)font->fpePrivate;
- fsfont =3D (FSFontPtr)font->fontPrivate;
     }
    =20
     /* make a new block record, and add it to the end of the list */
@@ -1793,7 +1822,7 @@
        err;
     int nranges =3D 0;
     int ret;
- fsRange *ranges, *nextrange =3D 0;
+ fsRange *nextrange =3D 0;
     unsigned long minchar, maxchar;
=20
     rep =3D (fsQueryXBitmaps16Reply *) fs_get_reply (conn, &ret);
@@ -1818,7 +1847,7 @@
     if (blockrec->type =3D=3D FS_LOAD_GLYPHS)
     {
  nranges =3D bglyph->num_expected_ranges;
- nextrange =3D ranges =3D bglyph->expected_ranges;
+ nextrange =3D bglyph->expected_ranges;
     }
=20
     /* place the incoming glyphs */
@@ -2185,7 +2214,7 @@
  xfree(ranges);
=20
  /* Now try to reopen the font. */
- return fs_send_open_font(client, (FontPathElementPtr)0,
+ return fs_send_open_font(client, pfont->fpe,
      (Mask)FontReopen, (char *)0, 0,
      (fsBitmapFormat)0, (fsBitmapFormatMask)0,
      (XID)0, &pfont);
@@ -2291,7 +2320,6 @@
 {
     FSFpePtr conn =3D (FSFpePtr) fpe->private;
     FSBlockDataPtr blockrec;
- FSBlockedListPtr blockedlist;
     int err;
=20
     /* see if the result is already there */
@@ -2302,7 +2330,6 @@
      err =3D blockrec->errcode;
      if (err =3D=3D StillWorking)
   return Suspended;
- blockedlist =3D (FSBlockedListPtr) blockrec->data;
      _fs_remove_block_rec(conn, blockrec);
      return err;
  }
@@ -3143,21 +3170,21 @@
 void
 fs_register_fpe_functions(void)
 {
- fs_font_type =3D RegisterFPEFunctions(fs_name_check,
- fs_init_fpe,
- fs_free_fpe,
- fs_reset_fpe,
- fs_open_font,
- fs_close_font,
- fs_list_fonts,
- fs_start_list_with_info,
- fs_next_list_with_info,
- (WakeupFpeFunc)fs_wakeup,
- fs_client_died,
- _fs_load_glyphs,
- NULL,
- NULL,
- NULL);
+ RegisterFPEFunctions(fs_name_check,
+ fs_init_fpe,
+ fs_free_fpe,
+ fs_reset_fpe,
+ fs_open_font,
+ fs_close_font,
+ fs_list_fonts,
+ fs_start_list_with_info,
+ fs_next_list_with_info,
+ fs_wakeup,
+ fs_client_died,
+ _fs_load_glyphs,
+ NULL,
+ NULL,
+ NULL);
 }
=20
 static int
@@ -3210,19 +3237,19 @@
 void
 check_fs_register_fpe_functions(void)
 {
- fs_font_type =3D RegisterFPEFunctions(fs_name_check,
- fs_init_fpe,
- fs_free_fpe,
- fs_reset_fpe,
- check_fs_open_font,
- fs_close_font,
- check_fs_list_fonts,
- check_fs_start_list_with_info,
- check_fs_next_list_with_info,
- (WakeupFpeFunc)fs_wakeup,
- fs_client_died,
- _fs_load_glyphs,
- NULL,
- NULL,
- NULL);
+ RegisterFPEFunctions(fs_name_check,
+ fs_init_fpe,
+ fs_free_fpe,
+ fs_reset_fpe,
+ check_fs_open_font,
+ fs_close_font,
+ check_fs_list_fonts,
+ check_fs_start_list_with_info,
+ check_fs_next_list_with_info,
+ fs_wakeup,
+ fs_client_died,
+ _fs_load_glyphs,
+ NULL,
+ NULL,
+ NULL);
 }

--KC+fneiph5CALyUl--

--1fZJyN7nFm/tosmV
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)

iEYEARECAAYFAkJEt0gACgkQ6kxmHytGonxgdQCbB4Tb8PdfWgy1Twco4xUZ4Ilr
fZYAoJdwNMrM3Yxdk7wi6uGMkbcCqXZx
=nLPN
-----END PGP SIGNATURE-----

--1fZJyN7nFm/tosmV--