On Fri, Jan 09, 2015 at 04:41:33PM +0200, Jani Nikula wrote:
> On Fri, 09 Jan 2015, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx
>
> 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@xxxxxxxxx>
>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html