Bug 32765

Summary: Xephyr segfaults on 24bpp hosts
Product: xorg Reporter: Michael Stone <michael>
Component: Server/DDX/XephyrAssignee: Matthew Allum <mallum>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: medium CC: ayers, hramrach, jeremyhu
Version: gitKeywords: patch
Hardware: Other   
OS: All   
See Also: https://launchpad.net/bugs/635523
https://bugzilla.redhat.com/show_bug.cgi?id=518960
https://qa.mandriva.com/show_bug.cgi?id=47928
https://bugs.freedesktop.org/show_bug.cgi?id=11053
Whiteboard: 2012BRB_Reviewed
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 44202    

Description Michael Stone 2010-12-31 17:34:24 UTC
For the past year, distro bugtrackers have been receiving reports that Xephyr segfaults when it tries to map its client-facing root window:

  https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/635523
  https://bugzilla.redhat.com/show_bug.cgi?id=518960
  https://qa.mandriva.com/show_bug.cgi?id=47928

The cause of the problem is that, on some 24bpp hosts, this computation:

  http://cgit.freedesktop.org/xorg/xserver/tree/hw/kdrive/ephyr/ephyr.c#n255

yields a value for priv->bytes_per_line which is too small. priv->bytes_per_line is then used by Xephyr to create its host-side image data buffer (resulting in a buffer that is too small). Then, when Xephyr maps its root window, it segfaults by writing beyond the end of the too-small image data buffer while filling its root window in response to expose-event/damage processing.

As for fixes: ajax proposed one fix for this problem six months ago that seems to have gotten lost after an unanswered request for an amendment by keithp:

  http://patchwork.freedesktop.org/patch/1327/

I tested this patch against Ubuntu's xserver-xorg_2:1.9.0-0ubuntu7 package (from Maverick) and can confirm that it fixed the segfault for me in that environment.
Comment 1 Dennis Sheil 2011-04-07 18:38:26 UTC
Still seeing this bug in Ubuntu 11.04 (Natty, not Maverick).

The definition of the KdScreenInfo data structure changed between the time of the aforementioned patch and now, so the lines:

screen->fb[0].depth
screen->fb[0].bitsPerPixel

should be:

screen->fb.depth
screen->fb.bitsPerPixel

I believe Keith Packard wanted a more robust patch, with the settings being obtained from the underlying XImage.  But my individual need is smaller, and Xephyr is segfaulting for me, so the patch is "good enough for me".  I applied the patch (with the modifications I mentioned) and Xephyr is now launching properly.
Comment 2 Jeremy Huddleston Sequoia 2011-09-18 01:18:18 UTC
I think we should just remove Xephyr in 1.12 now that we have xf86-video-nested
Comment 3 Julien Cristau 2011-09-18 08:29:28 UTC
On Sun, Sep 18, 2011 at 01:18:20 -0700, bugzilla-daemon@freedesktop.org wrote:

> --- Comment #2 from Jeremy Huddleston <jeremyhu@freedesktop.org> 2011-09-18 01:18:18 PDT ---
> I think we should just remove Xephyr in 1.12 now that we have xf86-video-nested

That seems rather premature to me.
Comment 4 Jeremy Huddleston Sequoia 2011-09-18 12:31:59 UTC
Why not?  We've been talking about it for 3-4 years now.  How long does something need to be unmaintained and bitrot before you decide to move on to its replacement?
Comment 5 David Ayers 2011-09-18 23:12:49 UTC
Has xf86-video-nested been packaged and backported to the stable/LTS releases of the major distributions?  If not I would agree that closing this issue is premature.
Comment 6 Jeremy Huddleston Sequoia 2011-09-18 23:33:09 UTC
xf86-video-nested is not in the LTS distros, but neither is xserver-1.12.
Comment 7 Jeremy Huddleston Sequoia 2011-09-18 23:34:58 UTC
I'm simply advocating that for 1.12 and onward, we should not advocate use of Xnest, Xvfb, Xfake, and Xephyr and instead advocate use of this alternative.  This will allow us to not split our efforts across three products which do the exact same thing going forward.

If you want to officially deprecate it in 1.12 and remove it in 1.13, I'm happy with that as well.
Comment 8 Adam Jackson 2018-06-11 21:29:54 UTC
commit 97cf53cc2ad7ecfdd495133bad31d0ec7d939326
Author: Søren Sandmann Pedersen <ssp@redhat.com>
Date:   Mon Oct 21 16:58:54 2013 -0400

    ephyr: hostx_screen_init(): Fix bits_per_pixel and bytes_per_line
    
    When the depth of the Xephyr server matches that of the host X server,
    Xephyr simply uses the buffer associated with the XImage as its
    framebuffer. In this case, it is correct to get the bits_per_pixel and
    bytes_per_line values returned from hostx_screen_init() from the XImage.
    
    However, when the depth doesn't match the host, Xephyr uses a private
    framebuffer that is periodically copied to the XImage. In this case,
    the returned values of bits_per_pixel and bytes_per_line should be
    those of the private framebuffer, not those of the XImage.
    
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Signed-off-by: Soren Sandmann <ssp@redhat.com>
    Reviewed-by: Adam Jackson <ajax@redhat.com>

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.