diff mbox series

[4/7] drm/i915/sdvo: Check that we have space for the infoframe

Message ID 20190409144054.24561-5-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Fix SDVO HDMI audio | expand

Commit Message

Ville Syrjälä April 9, 2019, 2:40 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Before we go writing the infoframe let's make sure we have
the space for it. Not that it really matters since the write
loop would just terminate early in that case.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Chris Wilson April 9, 2019, 7:46 p.m. UTC | #1
Quoting Ville Syrjala (2019-04-09 15:40:51)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Before we go writing the infoframe let's make sure we have
> the space for it. Not that it really matters since the write
> loop would just terminate early in that case.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 7f64352a3413..1e0102f1710f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
>                                   &hbuf_size, 1))
>                 return false;
>  
> +       if (hbuf_size < length)
> +               return false;
> +

Maybe after reporting the hbuf_size and length in the following
DRM_DEBUG_KMS?
-Chris
Ville Syrjälä April 9, 2019, 7:59 p.m. UTC | #2
On Tue, Apr 09, 2019 at 08:46:42PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-09 15:40:51)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Before we go writing the infoframe let's make sure we have
> > the space for it. Not that it really matters since the write
> > loop would just terminate early in that case.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 7f64352a3413..1e0102f1710f 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
> >                                   &hbuf_size, 1))
> >                 return false;
> >  
> > +       if (hbuf_size < length)
> > +               return false;
> > +
> 
> Maybe after reporting the hbuf_size and length in the following
> DRM_DEBUG_KMS?

Yes, that does seem more sensible. Avoids the off by one as well.
Chris Wilson April 9, 2019, 9:08 p.m. UTC | #3
Quoting Ville Syrjälä (2019-04-09 20:59:43)
> On Tue, Apr 09, 2019 at 08:46:42PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2019-04-09 15:40:51)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Before we go writing the infoframe let's make sure we have
> > > the space for it. Not that it really matters since the write
> > > loop would just terminate early in that case.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 7f64352a3413..1e0102f1710f 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
> > >                                   &hbuf_size, 1))
> > >                 return false;
> > >  
> > > +       if (hbuf_size < length)
> > > +               return false;
> > > +
> > 
> > Maybe after reporting the hbuf_size and length in the following
> > DRM_DEBUG_KMS?
> 
> Yes, that does seem more sensible. Avoids the off by one as well.

Ok, predictive
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7f64352a3413..1e0102f1710f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -970,6 +970,9 @@  static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 				  &hbuf_size, 1))
 		return false;
 
+	if (hbuf_size < length)
+		return false;
+
 	/* Buffer size is 0 based, hooray! */
 	hbuf_size++;