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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jan 09, 2015 at 04:41:33PM +0200, Jani Nikula wrote:
> 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.

Hmm, I somehow convinced myself that PORT_D wasn't in all of them.
I suppose I was thinking that port D isn't around on all platforms
and then thought the same will hold for the arrays without actually
checking. That does make the patch a bit less interesting.

> 
> > 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").
> 

Yeah I guess we can drop the stable then.

> 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

-- 
Ville Syrjälä
Intel OTC