Bug 15946 - Certain incantations of XDrawLines() can cause a server crash
Summary: Certain incantations of XDrawLines() can cause a server crash
Status: RESOLVED MOVED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: 7.3 (2007.09)
Hardware: x86 (IA32) Linux (All)
: medium normal
Assignee: Adam Jackson
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2008-05-15 08:47 UTC by Trond Kjernasen
Modified: 2018-12-13 22:19 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
test program that causes crash (1004 bytes, text/x-csrc)
2008-05-27 05:13 UTC, Peter Hutterer
no flags Details
patch (727 bytes, patch)
2009-12-24 06:03 UTC, Vladislav Zavjalov
no flags Details | Splinter Review

Description Trond Kjernasen 2008-05-15 08:47:54 UTC
Hi,

I've come across what appears to be a bug in X.Org's implementation of XDrawLines(), which results in a server crash. It seems related to the cap style and join style that is used, since changing it to the join style to JoinMiter and cap style to CapButt makes it works as expected.

The following code can be used to reproduce the crash:

...
  int pen_width = 50;
  XGCValues vals;
  vals.line_width = pen_width;
  vals.line_style = LineOnOffDash;
  vals.join_style = JoinBevel;
  vals.cap_style = CapProjecting;
  GC gc = XCreateGC(dpy, drawable,
                    GCLineWidth | GCLineStyle
                    | GCJoinStyle | GCCapStyle, &vals);

  char dashes[2];
  int dash_len = 2;
  dashes[0] = pen_width*4;
  dashes[1] = pen_width*2;
  XSetDashes(dpy, gc, 0, dashes, dash_len);

  XPoint points[5];
  points[0].x = 141; points[0].y = 40;
  points[1].x = 488; points[1].y = -160;
  points[2].x = 638; points[2].y = 100;
  points[3].x = 291; points[3].y = 300;
  points[4].x = 141; points[4].y = 40;

  XDrawLines(dpy, drawable, gc, points, 5, CoordModeOrigin);
  XFreeGC(dpy, gc);
...

--
Best Regards,
Trond Kjernaasen
Comment 1 Peter Hutterer 2008-05-27 05:11:06 UTC
Verified with git master, see backtrace:

Program received signal SIGABRT, Aborted.
[Switching to Thread -1208375616 (LWP 26833)]
0x00110402 in __kernel_vsyscall ()
(gdb) bt
#0  0x00110402 in __kernel_vsyscall ()
#1  0x00ba7690 in raise () from /lib/libc.so.6
#2  0x00ba8f91 in abort () from /lib/libc.so.6
#3  0x00bdf9eb in __libc_message () from /lib/libc.so.6
#4  0x00be7ac1 in _int_free () from /lib/libc.so.6
#5  0x00beb0f0 in free () from /lib/libc.so.6
#6  0x08166b40 in Xfree (ptr=0x9e87670) at utils.c:1422
#7  0x0814fa92 in miFillPolyHelper (pDrawable=0x9f2f800, pGC=0x9f2f5d0, 
    pixel=0, spanData=0x0, y=74, overall_height=31, left=0xbffa6ad8, 
    right=0xbffa6abc, left_count=1, right_count=0) at miwideline.c:170
#8  0x08152705 in miLineProjectingCap (pDrawable=0x9f2f800, pGC=0x9f2f5d0, 
    pixel=0, spanData=0x0, face=0xbffa6c08, isLeft=1, xorg=0, yorg=0, isInt=1)
    at miwideline.c:1263
#9  0x08154b75 in miWideDash (pDrawable=0x9f2f800, pGC=0x9f2f5d0, mode=0, 
    npt=1, pPts=0x9f0f2dc) at miwideline.c:2174
#10 0x003e4bda in fbPolyLine (pDrawable=0x9f2f800, pGC=0x9f2f5d0, mode=0, 
    npt=5, ppt=0x9f0f2cc) at fbline.c:140
#11 0x002c3794 in ExaCheckPolylines (pDrawable=0x9f2f800, pGC=0x9f2f5d0, 
    mode=0, npt=5, ppt=0x9f0f2cc) at exa_unaccel.c:176
#12 0x002ba28d in exaPolylines (pDrawable=0x9f2f800, pGC=0x9f2f5d0, mode=0, 
    npt=5, ppt=0x9f0f2cc) at exa_accel.c:685
#13 0x081bbfe1 in damagePolylines (pDrawable=0x9f2f800, pGC=0x9f2f5d0, mode=0, 
    npt=5, ppt=0x9f0f2cc) at damage.c:993
---Type <return> to continue, or q <return> to quit---
#14 0x0808a018 in ProcPolyLine (client=0x9f3bff0) at dispatch.c:1676
#15 0x08086bd6 in Dispatch () at dispatch.c:448
#16 0x0806cb64 in main (argc=8, argv=0xbffa6f84, envp=0xbffa6fa8) at main.c:415
Comment 2 Peter Hutterer 2008-05-27 05:13:02 UTC
Created attachment 16762 [details]
test program that causes crash

Trond's code as a compile-able c file.
Comment 3 Vladislav Zavjalov 2009-12-24 06:02:46 UTC
on crash miFillPolyHelper() is called from miLineProjectingCap() with the following parameters:

pixel:           1
spanData:        0
y:               11
overall_height:  6
left count:      2
right count:     2
lefts[0].height: -3
lefts[0].x: 62
lefts[0].stepx: 1
lefts[0].signdx: 1
lefts[0].e: -2
lefts[0].dy: 4
lefts[0].dx: 2
rights[0].height: 6
rights[0].x: 59
rights[0].stepx: 0
rights[0].signdx: -1
rights[0].e: -5
rights[0].dy: 6
rights[0].dx: 4
lefts[1].height: 9
lefts[1].x: 55
lefts[1].stepx: 0
lefts[1].signdx: -1
lefts[1].e: -3
lefts[1].dy: 6
lefts[1].dx: 4
rights[1].height: 0
rights[1].x: 48
rights[1].stepx: 1
rights[1].signdx: 1
rights[1].e: -1
rights[1].dy: 4
rights[1].dx: 2

The error is in negative lefts[0].height and wrong overall_height.

-----

miLineProjectingCap() runs with
face->dx: -6
face->dy: -4

There was a similar bug when dx=0, dy<0 led to a negative heigth:
https://bugs.freedesktop.org/show_bug.cgi?id=2690

Not sure I understand all the intricacies of these algorithms,
but it seems to me that the following patch is correct.
Comment 4 Vladislav Zavjalov 2009-12-24 06:03:43 UTC
Created attachment 32284 [details] [review]
patch
Comment 5 Corbin Simpson 2010-03-27 05:40:50 UTC
Tagging patch; will triage later.
Comment 6 debguy 2014-11-28 16:08:08 UTC
i don't see issuing a patch if you don't know the inners of the algorithm

also what is "git(1) verified" ?  never heard of it, is it any good ?

what i see is free() crashing

that means a memory address of the ptr that was freed which was not the same addres the original address that was allocated, or that the chain (the btree) of allocatons on the heap conflict (ie, maybe address wasn't in btree, or duplicate, etc).  ie, one cannot have alloc(ptr1); ptr2=ptr1; free(ptr2);  seg faults.

-------------------------------
it "should be true" that X server checks all X client requests are reasonable, ie, that any bogus pointers passed to X server are good.  however if X server ran lint on every ptr and graphics call - it'd be mighty slow.

perhaps let's look further into this and first determine if this is a seg fault

i dont' see here you saying if X server or X client crashed

i'm preeety sure than anything emulating hardware by drawing lines (noting good hw accel cards have postscript curve code in silicon).  that's all on X client side.  server side is where harware calls are made once client makes up it's mind what it has to say.

---------------------------
your code does not show where dpy is from or whether it's ok to free

i'm unsure if dpy is not conditional upon success, or if you are supposed to call client or server or test dpy before freeing.  sorry i couldn't be more help.

now are you SURE there aren't applications using Drawlines out there actually working which do not have this bug, that it really applies to X and not just your test ?

have fun !  jh
Comment 7 Vladislav Zavjalov 2014-11-29 05:27:04 UTC
> git(1) verified" ?  never heard of it

$ man git
git - the stupid content tracker
...

> i dont' see here you saying if X server or X client crashed

I guess this should be clear from the title or the trace log. Server crashes.
Comment 8 GitLab Migration User 2018-12-13 22:19:59 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/370.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.