| Summary: | xorg-server crashes in record when DRI2 is sending asynchronous reply to WaitMSC because client->requestBuffer has been already freed | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | xorg | Reporter: | Hannu Leinonen <hleinone> | ||||||
| Component: | Server/Ext/DRI | Assignee: | Xorg Project Team <xorg-team> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> | ||||||
| Severity: | critical | ||||||||
| Priority: | medium | CC: | hleinone, jeremyhu, rami.ylimaki, zd0929 | ||||||
| Version: | 7.6 (2010.12) | Keywords: | have-backtrace | ||||||
| Hardware: | x86-64 (AMD64) | ||||||||
| OS: | Linux (All) | ||||||||
| URL: | http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=646763 | ||||||||
| Whiteboard: | 2011BRB_Reviewed | ||||||||
| i915 platform: | i915 features: | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 40982 | ||||||||
| Attachments: |
|
||||||||
|
Description
Hannu Leinonen
2011-05-07 02:37:41 UTC
static void
RecordAReply(CallbackListPtr *pcbl, pointer nulldata, pointer calldata)
{
RecordContextPtr pContext;
RecordClientsAndProtocolPtr pRCAP;
int eci;
int majorop;
ReplyInfoRec *pri = (ReplyInfoRec *)calldata;
ClientPtr client = pri->client;
REQUEST(xReq); // <- gets stuff from freed pointer
majorop = stuff->reqType; // <- crash
Same in asm:
2820: 41 57 push %r15
2822: 41 56 push %r14
2824: 41 55 push %r13
2826: 49 89 d5 mov %rdx,%r13
2829: 41 54 push %r12
282b: 55 push %rbp
282c: 53 push %rbx
282d: 48 83 ec 28 sub $0x28,%rsp
2831: 4c 8b 3a mov (%rdx),%r15
// client = pri->client
2834: 49 8b 47 08 mov 0x8(%r15),%rax
// stuff = client->requestBuffer
2838: 44 0f b6 30 movzbl (%rax),%r14d
// majorop = stuff->reqType
client->requestBuffer doesn't hold any more original request because
reply is only send from WakeupHandler that is handling drmWaitVBlank
event for DRI2WaitMSC.
I don't know record&callback system enough to figure out how to fix the
crash quickly. It feels a bit like needing larger refactoring.
Is this a regression? *** Bug 42475 has been marked as a duplicate of this bug. *** (In reply to comment #2) > Is this a regression? No. It is old bug that was exposed by loosely related changes. The bug is in record callback that happens to be hit by DRI2 because it is using IgnoreClient and later sending reply from WakeupHandler. I guess simple fix for DRI2 caused crash would be changing WaitMSC to reset the current request. Later on wakeup handler would simple attend the client to handle same WaitMSC again. But that would still leave record crashing for any other asynchronous reply. Created attachment 53208 [details]
Failed attempt to write piglit test case for the crash
(In reply to comment #5) > But that would still leave record crashing for any other asynchronous reply. In my opinion option 1 from http://lists.x.org/archives/xorg-devel/2011-October/026017.html is the best way to fix this problem. + very easy it implement (just store op-codes and length in ClientRec and use them in RecordAReply) + protects all problematic requests automatically (RecordEnableContext, ListFontsWithInfo, DRI2WaitMSC, ...) - ClientRec is amended with data that is only needed by Record extension I have spent some time thinking of alternative approaches, but they are usually clumsy and require a lot more code fix the problem. I haven't provided a v2 patch yet, because I've been busy with other tasks lately. Based on an email from Julien, this should now be fixed in master and server-1.11-branch with fb22a408c69a84f81905147de9e82cf66ffb6eb2 |
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.