Launchpad has imported 12 comments from the remote bug at
https://bugs.freedesktop.org/show_bug.cgi?id=31501.

If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.

------------------------------------------------------------------------
On 2010-11-09T15:23:48+00:00 Jon TURNEY wrote:

Doing some investigation of this reported crash whilst using xserver
1.9.2 with xfs: http://cygwin.com/ml/cygwin-xfree/2010-11/msg00008.html,
I'm able to reproduce this crash with the following sequence of actions:

X &
xterm &
xset fp+ tcp/myfontserver:7100
xlsfonts

Program received signal SIGSEGV, Segmentation fault.
[Switching to thread 4712.0x1220]
0x005b2e29 in doListFontsAndAliases (client=0x125fdc0, c=0x120fce8) at 
dixfonts.c:611
611             fpe = c->fpe_list[c->current.current_fpe];
(gdb) p c
$1 = (LFclosurePtr) 0x120fce8
(gdb) p *c
$2 = {client = 0x120fce0, num_fpes = 18939104, fpe_list = 0x0, names = 0x0, 
current = {
    pattern = "▒W 
a\002\000\000\000ed-medium-r-normal--24-230-75-75-c-240-jisx0208.1983-0\va▒\207\va\000\000\000\000▒▒
 
\0019\017\000\000\b\000\000\000\000\000\000\000|▒%a\000\000\000\000\001\000\000\000\001\000\000\000▒\207\va\220▒\va@▒\va\000\000\000\000\030▒
 
\001\t\017\000\000\t\000\000\000\024\004\000\000▒▒%a\000\000\000\000\000\000\000\000\001\000\000\000\020\231\va▒\227\va▒\207\va\000\000\000\000H▒
 
\001▒\016\000\000\v\000\000\000\024\004\000\000▒▒%a\000\000\000\000\000\000\000\000\001\000\000\000"...,
 patlen = 62, current_fpe = 5, max_names = 3, list_started = 0, private = 
0x12df550},
  saved = {
    pattern = 
"*\000\000\000\001\000\000\000\020\231\va▒\227\va▒\207\va\000\000\000\000▒&\001i\017\000\000\005\000\000\000\024\004\000\000|▒#a\000\000\000\000\000\000\000\000\001\000\000\000\020\231\va▒\227\va▒\207\va\000\000\000\000▒▒
 
\0019\017\000\000\b\000\000\000\000\000\000\000|▒%a\000\000\000\000\001\000\000\000\001\000\000\000▒\207\va\220▒\va@▒\va\000\000\000\000\030▒
 
\001\t\017\000\000\t\000\000\000\024\004\000\000▒▒%a\000\000\000\000\000\000\000\000\001\000\000\000\020\231\va▒\227\va▒\207\va\000\000\000\000H▒
 
\001▒\016\000\000\v\000\000\000\024\004\000\000▒▒%a\000\000\000\000\000\000\000\000\001\000\000\000"...,
 patlen = 1, current_fpe = 0, max_names = 65535, list_started = 1, private = 
0x125fb10},
  haveSaved = 1, savedName = 0x12df338 
"-jis-fixed-medium-r-normal--24-230-75-75-c-240-jisx0208.1983-0",
  savedNameLen = 64}
(gdb)

On further investigation, the reason for the closure c being corrupt
seems to be related to the changes added in commit 3ab6cd31 to fix bug
#3040.

In doListFontsAndAliases(), if we get a Suspended result when the client
is already sleeping with the same closure (I don't really understand
enough what the code is doing to know if that's expected or not), then
the xinerama_sleep code frees() the closure c, so when it next wakes,
the closure c is being used after being freed.

Being a use after free bug possibly explains why the original reporter
is able to avoid the crash by changing the sequence of actions slightly.

The crash is not observed after reverting the noted commit, or with
xserver 1.8.2

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/0

------------------------------------------------------------------------
On 2010-12-09T22:52:05+00:00 Jon TURNEY wrote:

See also https://bugzilla.redhat.com/show_bug.cgi?id=658587

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/1

------------------------------------------------------------------------
On 2010-12-10T00:27:42+00:00 Jajones wrote:

Created attachment 40973
Possible fix

Haven't tested this thoroughly, as I'm not convinced early-out is really
the right way to handle Xinerama here, but I think this should fix the
crashes at least.  Feel free to try it out.

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/2

------------------------------------------------------------------------
On 2010-12-15T12:44:50+00:00 Colin-harrison wrote:

Hi,

It's not pretty, but James' patch fixes the crash for me.

Thanks,
Colin Harrison

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/3

------------------------------------------------------------------------
On 2011-03-07T12:12:14+00:00 Julien Cristau wrote:

*** Bug 31675 has been marked as a duplicate of this bug. ***

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/4

------------------------------------------------------------------------
On 2011-04-11T21:25:14+00:00 Jeremy Huddleston wrote:

James, have you sent this to xorg-devel for review / comment?  I didn't
see it in a quick check of emails from you in December.

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/22

------------------------------------------------------------------------
On 2011-04-27T08:09:38+00:00 Julien Cristau wrote:

*** Bug 36060 has been marked as a duplicate of this bug. ***

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/23

------------------------------------------------------------------------
On 2011-04-30T12:03:34+00:00 Acelists wrote:

*** Bug 34007 has been marked as a duplicate of this bug. ***

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/24

------------------------------------------------------------------------
On 2011-06-03T16:26:27+00:00 Cyril Brulebois wrote:

Here's another ping for this bad crasher. :)

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/25

------------------------------------------------------------------------
On 2011-07-29T18:23:36+00:00 Jeremy Huddleston wrote:

I've sent it to xorg-devel.  Please review and comment in that thread

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/26

------------------------------------------------------------------------
On 2011-07-29T20:13:18+00:00 Jajones wrote:

Discussion regarding why my patch ("Possible fix"), and a proposed patch
from Adam Jackson, aren't functionally correct from a long time ago:

< jamesjones> ajax: Did that patch I attached to the font loading crash bug 
look at all correct?
< ajax> oh man, i seriously just finished writing a patch for that
< ajax> if you beat me to it i'm going to feel very lame
< jamesjones> sorry
< jamesjones> I'm not at all sure it's correct.
< ajax> oh wow, i see what you did.
< jamesjones> It seems like the whole idea of bailing out if you need to sleep 
ruins the point of Xinerama looping
< ajax> yeah, i don't think that's right
< ajax> one second...
< jamesjones> but I was too lazy to dig deeper since that patch was enough to 
unblock my testing.
< ajax> jamesjones: give this one a try? http://fpaste.org/BqJf/raw/
< ajax> i'm a bit ill at how that whole thing works, honestly.
< jamesjones> Give me a second, rebuilding
< jamesjones> But yeah, it doesn't make sense to me.
< ajax> at any rate it fixes my testcase of xset fp+ unix/:7100 && xlsfonts
< jturney> heh, it removes code, so it must be right :)
< jamesjones> Hehe
< ajax> nothing like having a library where your app has to implement functions 
with particular names and signatures to work
< jamesjones> but why the looping, if it's OK to return Success when the 
operation isn't done on screen[n] where n>0?
< ajax> jamesjones: it's not quite that.
< ajax> most of the font ops don't have any screen state, so you only have to 
call them once.
< ajax> PolyText and ImageTexto sleep do have to hit every screen, so you want 
to make sure that they only bump the client's sleep count once if they d
< ajax> so the minimal thing would have been to only change the closures for 
{Poly,Image}Text to not keep a sleep count
< ajax> but if you're doing that you may as well do them all
< jamesjones> right, I figured it was something like that, but then 1) Why loop 
the others 2) Why is it OK to early-out in PolyText/ImageText?  Does the 
looping get restarted somewhere I'm not seeing when the client wakes up?
< jamesjones> I suppose I should just load up xinerama and step through it
< ajax> the others aren't actually looped by xinerama
< ajax> (which i had missed the first time around, i just assumed they were)
< jamesjones> ah, k
< jamesjones> that makes more sense.
< ajax> the text rendering routines push themselves onto the work queue if they 
don't have a font they need because the font server hasn't gotten back to them
< ajax> check out the context around ClientSleep in doImageText, it's quite 
horrific
< ajax> the way they wake up is that the font server's fd selects readable, and 
the handler for that notices which request tat reply was for and pokes the 
client back into the work queue
< ajax> i think that's right anyway?  i kind of want to not know.
< jamesjones> it sounds right if there's only one screen
< ajax> well, if there's multiple screens, you might run through the sleep 
pattern multiple times
< jamesjones> Yeah, but it looks like each screen would loop in and block, only 
sleeping once
< jamesjones> then when it woke up, only the first screen would be serviced.
< jamesjones> *loop in and detect the need to sleep rather
< ajax> ew.  yeah.
< ajax> okay, so
< jamesjones> Also, it seems like with your patch, those other screens would 
leak their closure structs
< ajax> i think the way to fix that is to fix PanoramiX*Text* check for sleep 
too
< jamesjones> which is what I was attempting to avoid in mine.
< jamesjones> Yeah, that sounds like it has a better chance at correctness.
< jamesjones> Or I was thinking something like looping at a lower level, so 
there would be xinerama data in teh closure, but that sounded like it would 
violate all kinds of design stuff
< jamesjones> Your patch does avoid the crash I was seeing, FWIW
< ajax> progress
< ajax> thanks for testing.  i'll fix the xinerama thing and send the patches 
off to the list
< jamesjones> np, sounds good
< ajax> every once in a while i dream about working in a language where 
closures aren't something you have to hand roll every time
< jamesjones> If there weren't a bunch entrenched old languages I happen to 
know, I'd have to do something creative with my time.  Couldn't have that.
< ajax> aaaaaargh
< ajax> if you change PanoramiX*Text* to be restartable then there's a race 
between each screen where some other client on the same drawable or gc could 
modify your state
< ajax> so there goes the atomicity invariant

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/27

------------------------------------------------------------------------
On 2012-06-14T19:39:54+00:00 Julien Cristau wrote:

*** Bug 48900 has been marked as a duplicate of this bug. ***

Reply at: https://bugs.launchpad.net/xorg-server/+bug/738526/comments/30


** Changed in: xorg-server
   Importance: Medium => Critical

-- 
You received this bug notification because you are a member of Ubuntu-X,
which is subscribed to xorg-server in Ubuntu.
https://bugs.launchpad.net/bugs/738526

Title:
  crash accessing font info with xfs in fontpath

To manage notifications about this bug go to:
https://bugs.launchpad.net/xorg-server/+bug/738526/+subscriptions

_______________________________________________
Mailing list: https://launchpad.net/~ubuntu-x-swat
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~ubuntu-x-swat
More help   : https://help.launchpad.net/ListHelp

Reply via email to