Bug 36119 - xorg-server 1.10.0.902 sigbuses in Xi/exevents.c:UpdateDeviceState() while accessing v->axisVal[0] for second time
Summary: xorg-server 1.10.0.902 sigbuses in Xi/exevents.c:UpdateDeviceState() while ac...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Peter Hutterer
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: xserver-1.10
  Show dependency tree / graph
 
Reported: 2011-04-10 14:12 UTC by Petr Pisar
Modified: 2011-05-28 17:32 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

Attachments

Description Petr Pisar 2011-04-10 14:12:42 UTC 
Since xorg-server 1.10.0.901, I get SIGBUS when moving core pointer on n32 MIPS64 Linux. The architecture requires aligned access to memory. The aborting signal is emitted when accessing v->axisVal[0] in UpdateDeviceState() from Xi/exevents.c.

I found out the v->axisVal[0] (a double implemented as 8 bytes) variable is aligned for the first time, but unaligned on second function call. I identified the problem in DeepCopyPointerClasses() where continuous memory block for valuator and valuator->axes and valuator->axisVal is reallocated:

        to->valuator = realloc(to->valuator, sizeof(ValuatorClassRec) +
                from->valuator->numAxes * sizeof(AxisInfo) +
                from->valuator->numAxes * sizeof(double));
        v = to->valuator;
        if (!v)
            FatalError("[Xi] no memory for class shift.\n");

        v->numAxes = from->valuator->numAxes;
        v->axes = (AxisInfoPtr)&v[1];
        memcpy(v->axes, from->valuator->axes, v->numAxes * sizeof(AxisInfo));

        v->axisVal = (double*)(v->axes + from->valuator->numAxes);

Here you can see the v->axisVal is explicitly cast to (double *) from offset computed as concatenation of v and AxisInfo * numAxes. Unfortunately sizeof(struct _ValuatorClassRec) is not divisible by 8, thus v->axisVal is placed to non-aligned address and causes SIGBUS on later write access in UpdateDeviceState().

I verified that putting appropriate padding into struct _ValuatorClassRec (one unsigned short after numAxes and one double* after axisVal) fixes the problem for me.

Unfortunately I do not know how to fix the alignment correctly in portable way.
Comment 1 Jeremy Huddleston Sequoia 2011-04-11 13:47:15 UTC 
Petr, you said "since 1.10.0.901" ... can you please verify that 1.10.0 does not have this behavior?  exevents.c did not change between 1.10.0 and 1.10.0.901 ... 

The only changes between 1.10.0 and 1.10.0.901 which might be related are these two:

http://cgit.freedesktop.org/xorg/xserver/commit/?h=server-1.10-branch&id=09f6d85b5bb49406f015ec667bb6efb3e710ced2
http://cgit.freedesktop.org/xorg/xserver/commit/?h=server-1.10-branch&id=a96fd08b406b9955ff0d5c02f4a50f4fb2acf40e

While certainly not the cause of the problem, it's possible that it just worked before by luck, and fixing those two issues rearranged memory to poke n32 in a bad place.

Petr, can you please try reverting those to changes to see if your problem goes away?
Comment 2 Peter Hutterer 2011-04-11 15:06:29 UTC 
(In reply to comment #1)
> Petr, you said "since 1.10.0.901" ... can you please verify that 1.10.0 does
> not have this behavior?  exevents.c did not change between 1.10.0 and
> 1.10.0.901 ... 

commit 678f5396c91b3d0c7572ed579b0a4fb62b2b4655
Refs: xorg-server-1.9.99.903-5-g678f539

fixed this problem during init but DeepCopyDeviceClasses still has the same issue when the valuator numbers differ. I'll get a fix out, but this bug should be visible with any 1.10 version
Comment 3 Jeremy Huddleston Sequoia 2011-04-11 15:26:52 UTC 
Ok, based on this bugs existence in 1.10.0, it will not block 1.10.1.  However, if a fix is available by Wednesday with minimal code change, I will merge it in to allow testing before final release on Friday.  If the change is too large or otherwise risky, I'll bring it in after 1.10.0.
Comment 4 Jeremy Huddleston Sequoia 2011-04-11 15:27:48 UTC 
The last sentence should've ended with "after 1.10.1."
Comment 5 Peter Hutterer 2011-04-11 18:06:40 UTC 
http://patchwork.freedesktop.org/patch/4924/
Comment 6 Petr Pisar 2011-04-12 10:30:00 UTC 
I said since because that for first version I observed the problem. Version I used before was 1.9.5.

The patch by Peter works for me.
Comment 7 Jeremy Huddleston Sequoia 2011-04-12 11:08:49 UTC 
Ok, since this issue is present in 1.10.0, it does not meet the criteria to merge into 1.10.1 at this point.  The patch is scheduled to be pulled into 1.10 during the 1.10.2 cycle after a sufficient test period on master.
Comment 8 Jeremy Huddleston Sequoia 2011-05-28 17:32:25 UTC 
This was fixed in 1.10.2 RC1

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.