Created attachment 80090 [details] return NULL for xf86CompatOutput if config->compat_output < 0 Solves the xserver/hw/xfree86/modes compat_output == -1 bad dereference. The commit 37d956e3ac9513b74078882dff489f9b0a7a5a28 into xf86Crtc.c xf86CrtcConfigInit() initializes config->compat_output = -1; So an unset compat_output is now invalid, a good property since the previous initial compat_output == 0 is not guaranteed available. It usually is available because a driver first calls xf86OutputCreate() followed by xf86InitialConfiguration(). This change exposes a bug. During initial configuration a monitor detection is attempted and if an EDID block is available the driver calls xf86OutputSetEDID(). There a bad dereference occurs: xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow) -> xf86ProbeOutputModes(scrn, width, height); --> output_modes = (*output->funcs->get_modes) (output); ---> xf86OutputSetEDID(xf86OutputPtr output, xf86MonPtr edid_mon) ----> if (output == xf86CompatOutput(scrn)) XX now bad dereference -----> return config->output[config->compat_output]; XX output[-1] ----> xf86SetDDCproperties(scrn, edid_mon); XX for screen monitor -> xf86SetScrnInfoModes(scrn); --> output = SetCompatOutput(config); XX sets compat_output ---> config->compat_output = compat; Hence previously the screen monitor properties were always copied from output 0 since compat_output is only set at the end of above function. Two old Xorg.log files illustrate the previous behaviour. The suggestion > If there is no compat output, config->compat_output is -1 and xf86CompatOutput > reads before the beginning of the outputs array. > Signed-off-by: Aaron Plattner <aplattner@nvidia.com> > diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h > index 802303f..1ac8485 100644 > --- a/hw/xfree86/modes/xf86Crtc.h > +++ b/hw/xfree86/modes/xf86Crtc.h > @@ -731,6 +731,8 @@ xf86CompatOutput(ScrnInfoPtr pScrn) > { > xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn); > > + if (config->compat_output < 0) > + return NULL; > return config->output[config->compat_output]; > } Tested-by: Servaas Vandenberghe avoids the bad dereference. http://lists.x.org/archives/xorg-devel/2013-March/035751.html http://lists.x.org/archives/xorg-devel/2013-April/035933.html
Created attachment 80091 [details] [review] return NULL for xf86CompatOutput if config->compat_output < 0
Created attachment 80093 [details] Xorg 2010 log: EDID monitor connected at output 0. During initial configuration compat_output == 0 so xf86SetDDCproperties() is called. Search for 'Printing DDC '.
Created attachment 80094 [details] Xorg 2010 log: EDID monitor connected at output 2. Since compat_output == 0 during initial configuration xf86SetDDCproperties() is never called. Search for 'EDID'.
Just FYI, patches mailed to xorg-devel, as described on http://www.x.org/wiki/Development/Documentation/SubmittingPatches get applied to the X server sources with an average of several years less delay than those stuck in bugzilla. (Sorry, we're overloaded and don't have developers to spare going through bugzilla looking for patches.)
commit 28159eff6badf6181b255f26d1f444abe81c05b7 Author: Jason Gerecke <killertofu@gmail.com> Date: Thu Apr 30 18:06:14 2015 -0700 xfree86: Return NULL from xf86CompatOutput if no compat_output is defined If no compat_output is defined, we inadvertently (attempt to) return whatever data is at index -1. Instead, return NULL since that's what callers are expecting. Reviewed-by: Adam Jackson <ajax@redhat.com> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.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.