On 11/08/2011 08:22 PM, Keith Packard wrote:
The record extension needs the major and minor opcodes in the reply
hook, but the request buffer may have been freed by the time the hook
is invoked. Saving the request major and minor codes as the request is
executed avoids fetching from the defunct request buffer.

This patch also eliminates the public MinorOpcodeOfRequest function,
making it static to dispatch. Usages of that function have been
replaced with direct access to the new ClientRec field.

Signed-off-by: Keith Packard<kei...@keithp.com>
---

Here's what I was thinking of to fix this -- just record the major and
minor opcodes of the request in the ClientRec during Dispatch and then
using those fields in RecordAReply instead of fetching the discarded
request buffer.

This is entirely untested; I don't know how to make the old code break.

Reviewed-by: Rami Ylimäki <rami.ylim...@vincit.fi>

This is the easiest and most reliable way to fix the problem. I tested the fix and it works.



Reproduction steps:

1. Run X server under Valgrind.
2. Record replies to an offending request (ListFontsWithInfo, RecordEnableContext, DRI2WaitMSC, ... ?). I used "cnee --record -erpmar 128-135" to record replies to Record extension as I know that the major code of that extension is somewhere in range 128-135 in my environment. 3. Run a client that executes the offending request in another shell. I used "cnee --record --mouse" as it executes RecordEnableContext.
4. Kill the client that was used in step 3. I just sent Ctrl+C to cnee.
5. Examine Valgrind log. With the above steps I can always see complaints about invalid reads when major-op is read from the freed request buffer. After the fix this specific complaint is gone.



This is unrelated but during testing I found a yet another bug in Record extension at least in 1.9.5 server:

1. In one shell: "cnee --record -repra 0-255"
2. In other shell "cnee --record --mouse"

X server will examine whether the range is valid, determines that it's not and crashes with an assert. This is unrelated to the fix.

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to