Re: [Intel-gfx] [PATCH 1/7] drm/i915: Make hpd arrays big enough to avoid out of bounds access

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 09 Jan 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_hpd_irq_handler() walks the passed in hpd[] array assuming it
> contains HPD_NUM_PINS elements. Currently that's not true as we don't
> specify an explicit size for the arrays when initializing them. Avoid
> the out of bounds accesses by specifying the size for the arrays.

My first impression was wowowow, this must fix some of those obscure hpd
bugs we seem to have. But no, all of the arrays have HPD_PORT_D which
means we don't go out of bounds. This patch doesn't change the sizes of
the arrays.

> Cc: stable@vger.kernel.org

I'm not sure if that's appropriate in light of the above ("It must fix a
real bug that bothers people").

But we should do this *before* it becomes a bug.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 818ab4e..1d52ae9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -45,7 +45,7 @@
>   * and related files, but that will be described in separate chapters.
>   */
>  
> -static const u32 hpd_ibx[] = {
> +static const u32 hpd_ibx[HPD_NUM_PINS] = {
>  	[HPD_CRT] = SDE_CRT_HOTPLUG,
>  	[HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
>  	[HPD_PORT_B] = SDE_PORTB_HOTPLUG,
> @@ -53,7 +53,7 @@ static const u32 hpd_ibx[] = {
>  	[HPD_PORT_D] = SDE_PORTD_HOTPLUG
>  };
>  
> -static const u32 hpd_cpt[] = {
> +static const u32 hpd_cpt[HPD_NUM_PINS] = {
>  	[HPD_CRT] = SDE_CRT_HOTPLUG_CPT,
>  	[HPD_SDVO_B] = SDE_SDVOB_HOTPLUG_CPT,
>  	[HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT,
> @@ -61,7 +61,7 @@ static const u32 hpd_cpt[] = {
>  	[HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT
>  };
>  
> -static const u32 hpd_mask_i915[] = {
> +static const u32 hpd_mask_i915[HPD_NUM_PINS] = {
>  	[HPD_CRT] = CRT_HOTPLUG_INT_EN,
>  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_EN,
>  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_EN,
> @@ -70,7 +70,7 @@ static const u32 hpd_mask_i915[] = {
>  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_EN
>  };
>  
> -static const u32 hpd_status_g4x[] = {
> +static const u32 hpd_status_g4x[HPD_NUM_PINS] = {
>  	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
>  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_G4X,
>  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_G4X,
> @@ -79,7 +79,7 @@ static const u32 hpd_status_g4x[] = {
>  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
>  };
>  
> -static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
> +static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are the same */
>  	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
>  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
>  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
> @@ -1522,7 +1522,7 @@ static inline enum port get_port_from_pin(enum hpd_pin pin)
>  static inline void intel_hpd_irq_handler(struct drm_device *dev,
>  					 u32 hotplug_trigger,
>  					 u32 dig_hotplug_reg,
> -					 const u32 *hpd)
> +					 const u32 hpd[HPD_NUM_PINS])
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int i;
> -- 
> 2.0.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center