re: xsrc/58133: X server crashes; radeon 5450; modesetting

[matthew green <mrg%eterna23.net@localhost>]

The following reply was made to PR xsrc/58133; it has been noted by GNATS.

From: matthew green <mrg%eterna23.net@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: xsrc-manager%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
    netbsd-bugs%netbsd.org@localhost
Subject: re: xsrc/58133: X server crashes; radeon 5450; modesetting
Date: Wed, 10 Apr 2024 14:44:59 +1000

 > glamor_text(DrawablePtr drawable, GCPtr gc,
 >             glamor_font_t *glamor_font,
 >             glamor_program *prog,
 >             int x, int y,
 >             int count, char *s_chars, CharInfoPtr *charinfo,
 >             Bool sixteen)
 > {
 > ...
 > 168     =
 
 > 169                 tx =3D (col - firstCol) * glyph_spacing_x;
 > 170                 /* adjust for second row layout */
 > 171                 tx +=3D second_row * glamor_font->row_width * 8;
 > 172     =
 
 > 173                 v[ 0] =3D x1;
 > 174                 v[ 1] =3D y1;
 > 175                 v[ 2] =3D width;
 > 176                 v[ 3] =3D height;
 > 177                 v[ 4] =3D tx;
 >
 > Line 173 looks like a plausible place for a segfault. My copy of this
 > file has not been modified since 2016.
 >
 > 0x13e28 is in glamor_image_text (/usr/xsrc/external/mit/xorg-server/dist=
 /glamor/glamor_text.c:470).
 > 465             goto bail;
 > 466     =
 
 > 467         (void) glamor_text(drawable, gc, glamor_font, prog,
 > 468                            x, y, count, chars, charinfo, sixteen);
 > 469     =
 
 > 470         return TRUE;
 > 471     =
 
 > 472     bail:
 > 473         return FALSE;
 > 474     }
 >
 > (gdb) list *(glamor_image_text8+0x30)
 > 0x13fa9 is in glamor_image_text8 (/usr/xsrc/external/mit/xorg-server/dis=
 t/glamor/glamor_text.c:480).
 > 475     =
 
 > 476     void
 > 477     glamor_image_text8(DrawablePtr drawable, GCPtr gc,
 > 478                        int x, int y, int count, char *chars)
 > 479     {
 > 480         if (!glamor_image_text(drawable, gc, x, y, count, chars, FAL=
 SE))
 > 481             miImageText8(drawable, gc, x, y, count, chars);
 > 482     }
 
 this looks like a problem i've seen once or twice.  i think that the
 v array is not properly set but i've not managed to figure out what
 is going wrong.  actually, there's a clear case where it seems bad,
 in that we also have:
 
 122     v =3D glamor_get_vbo_space(drawable->pScreen, count * (6 * sizeof =
 (GLshort)), &vbo_offset);
 
 but v is not checked and used at L173 after this call, and looking
 at glamor_get_vbo_space() it seems to be able to return NULL in the
 case that radeon_buffer_object{} has a NULL 'bo' member, which is
 initialisation point.
 
 if you can reproduce this easily, can you patch the above to do eg:
 
 	if (v =3D=3D NULL)
 		return x;
 
 after L122?
 
 
 .mrg.