[PATCH] xf86: return NULL for xf86CompatOutput if

vdb at picaros.org vdb at picaros.org
Tue Apr 9 01:00:11 PDT 2013

xserver/hw/modes: compat_output bad dereference.

The commit 37d956e3ac9513b74078882dff489f9b0a7a5a28 into 
xserver/hw/modes/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];
      ----> 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 the function.

The suggestion

> If there is no compat output, config->compat_output is -1 and
> reads before the beginning of the outputs array.

> Signed-off-by: Aaron Plattner <aplattner at 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.

More information about the xorg-devel mailing list