Yet another bug revelead by OpenBSD's malloc(). When the server quits, miDCCloseScreen() calls the destroyPixmap on pScreenPriv->pRootPicturs. But this pixmap has already been freed by PictureDestroyWindow() which was iterates over all Pictures owned by a window. The desproPixmap method tries to access the refcount field in the pixmap structure and causes a segmentation fault when the freed memory is not readable anymore. The easiest fix may be not to free this pixmap in miDCCloseScreen anymore. I'm attaching a patch that adds some traces to show the problem, even on systems that won't cause a segfault when accessing free memory.
Created attachment 3035 [details] [review] patch to trace the problem observe the following sequence in the X server output: pRootPicture: 0x80fe0f00 1 Freeing RootPicture <- this happens in PictureDestroyWindow() tossPict 0x80fe0f00 -791621424 <- and this in miDCCloseScreen() pRootPicture points to free memory.
This pixmap is represented in two locations. Therefore it's not easy to let the second call to free know that other location has already done the free. I agree with your suggested fix but we need to know if we may not run into situations where PictureDestroyWindow() doesn't get called for this window (I assume it's thre root window). Keith would probably know best - adding him to Cc:
Unfortunately the patch breaks the build of Xprt (Xorg server build works): gcc -o Xprt -O2 -fmessage-length=0 -Wall -g -fno-strict-aliasing -ansi -pedantic -Wall -Wpointer-arith +-Wundef -L../../exports/lib Xprint/ddxInit.o Xprint/miinitext.o Xprint/dpmsstubs.o +os/libcwrapper.o dix/libdix.a os/libos.a Xprint/libprinter.a Xprint/raster/libraster.a +Xprint/pcl/libpcl.a Xprint/pcl-mono/libpcl.a Xprint/ps/libps.a mfb/libmfb.a cfb/libcfb.a +cfb32/libcfb32.a mfb/libmfb.a dix/libxpstubs.a mi/libmi.a composite/libcomposite.a damageext/libdamage.a +miext/damage/libdamage.a xfixes/libxfixes.a miext/cw/libcw.a Xext/libexts.a xkb/libxkb.a +Xi/libxinput.a lbx/liblbx.a ../../lib/lbxutil/liblbxutil.a +randr/librandr.a render/librender.a Xext/libext.a dbe/libdbe.a record/librecord.a GL/glx/libglx.a +GL/mesa/GLcore/libGLcore.a XTrap/libxtrap.a ../../lib/font/libXfont.a -lfreetype dix/libxpstubs.a +-lz -lm -lusb -lresmgr -lXau -lXdmcp -Wl,-rpath-link,../../exports/lib Xprint/ps/libps.a(psout_ftpstype1.o): In function `PsOut_DownloadFreeType1': /usr/src/packages/BUILD/xc/programs/Xserver/Xprint/ps/psout_ftpstype1.c:77: warning: the use of `tempnam' +is dangerous, better use `mkstemp' render/librender.a(picture.o): In function `PictureDestroyWindow': /usr/src/packages/BUILD/xc/programs/Xserver/render/picture.c:116: undefined reference to +`miDCIsRootPicture' collect2: ld returned 1 exit status make[4]: *** [Xprt] Error 1
The patch is only there to hilight the problem. It's not a fix, so I didn't consider breaking Xprt as a big issue.
Created attachment 3162 [details] [review] This is a patch implementing the workaround I propose.
Thanks!
Comment on attachment 3162 [details] [review] This is a patch implementing the workaround I propose. looks sane, worst it could do is leak.
keith, correctness comments please.
Yeah, the patch looks correct; all pictures pointing at windows needn't get freed as they are freed when the window is destroyed.
-> 7.2
Committed now.
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.