| Summary: | X server crash in nouveau_xv.c:NVPutImage (NVCopyNV12ColorPlanes) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | xorg | Reporter: | Ilia Mirkin <imirkin> | ||||
| Component: | Driver/nouveau | Assignee: | Nouveau Project <nouveau> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> | ||||
| Severity: | normal | ||||||
| Priority: | medium | ||||||
| Version: | unspecified | ||||||
| Hardware: | x86-64 (AMD64) | ||||||
| OS: | Linux (All) | ||||||
| Whiteboard: | |||||||
| i915 platform: | i915 features: | ||||||
| Attachments: |
|
||||||
|
Description
Ilia Mirkin
2013-04-08 12:01:30 UTC
Reading the code some more, higher up in the call chain (ProcXv{Shm,}PutImage), a call to NVQueryImageAttributes is made to determine how many bytes the source image needs to be, and if there aren't enough bytes, we never even make it to NVPutImage.
So this means that we're looking for some sort of inconsistency between NVQueryImageAttributes and how far into the source buffer NVPutImage reads. Sadly the logic in NVPutImage is separate, and is much more complex.
By the way, it occurs to me that I didn't mention what hardware I'm using, in case it matters:
02:00.0 VGA compatible controller: NVIDIA Corporation G96 [GeForce 9500 GT] (rev a1)
And I'm using x11-libs/libdrm-2.4.40 (which nouveau_xv.c depends on via nouveau_bo_*)
A little more info: I added code to call NVQueryImageAttributes inside of NVPutImage, compute an end pointer (buf + size), and then check inside of NVCopyNV12ColorPlanes at the end of every loop iteration whether either us or vs have gone off the end. And it seems like they do! When I move the mplayer window s.t. part of it is off-screen (on the left), the code ends up accessing 2 bytes further than the end of the array! There happens to be another mapping afterwards which means that there's no segfault, but if that mapping isn't there, a segfault would have occurred. Now, it only ever goes over by 1-3 bytes, never more. One thing that I noticed is that we pass in line_len to NVCopyNV12ColorPlanes as the width (which is rounded up to 8 on NV_50 and up) rather than npixels (which is rounded up to 4). I also wonder if there's some issue in how left is computed (and then applied to the s2/3 offsets)... Looks like my npixels vs line_len guess was right: [741217.047] (WW) NOUVEAU(0): Inconsistency between NVPutImage and NVQueryImageAttributes: src_x: 0, src_y: 0, src_w: 512, src_h: 384, top: 0, left: 102, bottom: 384, right: 512, npixels: 410, nlines: 384, line_len: 416, srcPitch2: 256, s2offset: 196659, s3offset: 245811, bytes: 49104, size_tmp: 294912, over: 3 [741217.047] (WW) NOUVEAU(0): Went too far: us: 0x7f7e20b6c002, vs: 0x7f7e20b78002, end (non-incl): 0x7f7e20b78000 And the reason I only see it when the picture is off-screen is because normally the width is already a multiple of 8. Not sure what the right way of fixing it is though... I think we can just use npixels instead of line_len as the width parameter. However that will leave the last 1-3 pixels in the dst buffer uninitialized. Not sure if that's OK. You can leave the last few pixels uninitialized. The last few pixels already contained garbage, and it's up to the implementation to make sure they're not included when rendering the picture. :) Created attachment 78944 [details] [review] Use npixels instead of line_len for the width to avoid going too far Thank you for debugging and fixing this Ilia ^_^ Your patch has been pushed to master |
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.