[PATCH 0/3] drm: Fix/annotate better a few error paths

[PATCH 0/3] drm: Fix/annotate better a few error paths

[Imre Deak]

Fix a few minor issues durring error handling, all of these were pointed
at by Coverity reports.

Imre Deak (3):
  drm: Tune up error message during format->bpp/cpp conversion
  drm/mst: Fix error handling during MST sideband message reception
  drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req()

 drivers/gpu/drm/drm_crtc.c            | 10 +++++++---
 drivers/gpu/drm/drm_dp_mst_topology.c | 16 +++++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion

[Imre Deak]

Returning a 0 bpp/cpp value from these functions isn't ever valid. In
many cases it can also lead to a div-by-zero possibly at some later
point in time, so make sure we catch such errors as soon as possible via
louder error reporting.

CC: Dave Airlie <airlied@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 70f9c68..3a32606 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5610,8 +5610,8 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
 		*bpp = 32;
 		break;
 	default:
-		DRM_DEBUG_KMS("unsupported pixel format %s\n",
-			      drm_get_format_name(format));
+		WARN(1, "unsupported pixel format %s\n",
+		     drm_get_format_name(format));
 		*depth = 0;
 		*bpp = 0;
 		break;
@@ -5666,8 +5666,12 @@ int drm_format_plane_cpp(uint32_t format, int plane)
 	unsigned int depth;
 	int bpp;
 
-	if (plane >= drm_format_num_planes(format))
+	if (plane >= drm_format_num_planes(format)) {
+		WARN(1, "invalid plane %d for format %s\n",
+		     plane, drm_get_format_name(format));
+
 		return 0;
+	}
 
 	switch (format) {
 	case DRM_FORMAT_YUYV:
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception

[Imre Deak]

Handle any error due to partial reads, timeouts etc. to avoid parsing
uninitialized data subsequently. Also bail out if the parsing itself
fails.

CC: Dave Airlie <airlied@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index a13edf5..12c0ab3 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2204,11 +2204,19 @@ static void drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
 		ret = drm_dp_dpcd_read(mgr->aux, basereg + curreply,
 				    replyblock, len);
 		if (ret != len) {
-			DRM_DEBUG_KMS("failed to read a chunk\n");
+			DRM_DEBUG_KMS("failed to read a chunk (len %d, ret %d)\n",
+				      len, ret);
+
+			return;
 		}
+
 		ret = drm_dp_sideband_msg_build(msg, replyblock, len, false);
-		if (ret == false)
+		if (!ret) {
 			DRM_DEBUG_KMS("failed to build sideband msg\n");
+
+			return;
+		}
+
 		curreply += len;
 		replylen -= len;
 	}
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req()

[Imre Deak]

In case of an unknown broadcast message is sent mstb will remain unset,
so check for this.

CC: Dave Airlie <airlied@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 12c0ab3..412b9ca 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2334,7 +2334,9 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
 			DRM_DEBUG_KMS("Got RSN: pn: %d avail_pbn %d\n", msg.u.resource_stat.port_number, msg.u.resource_stat.available_pbn);
 		}
 
-		drm_dp_put_mst_branch_device(mstb);
+		if (mstb)
+			drm_dp_put_mst_branch_device(mstb);
+
 		memset(&mgr->up_req_recv, 0, sizeof(struct drm_dp_sideband_msg_rx));
 	}
 	return ret;
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion

[Ville Syrjälä]

On Thu, May 12, 2016 at 04:00:38PM +0300, Imre Deak wrote:
> Returning a 0 bpp/cpp value from these functions isn't ever valid. In
> many cases it can also lead to a div-by-zero possibly at some later
> point in time, so make sure we catch such errors as soon as possible via
> louder error reporting.
> 
> CC: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_crtc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 70f9c68..3a32606 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5610,8 +5610,8 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
>  		*bpp = 32;
>  		break;
>  	default:
> -		DRM_DEBUG_KMS("unsupported pixel format %s\n",
> -			      drm_get_format_name(format));
> +		WARN(1, "unsupported pixel format %s\n",
> +		     drm_get_format_name(format));

NAK. This will happen every time drm_helper_mode_fill_fb_struct()
is called with a non-RGB format.

>  		*depth = 0;
>  		*bpp = 0;
>  		break;
> @@ -5666,8 +5666,12 @@ int drm_format_plane_cpp(uint32_t format, int plane)
>  	unsigned int depth;
>  	int bpp;
>  
> -	if (plane >= drm_format_num_planes(format))
> +	if (plane >= drm_format_num_planes(format)) {
> +		WARN(1, "invalid plane %d for format %s\n",
> +		     plane, drm_get_format_name(format));
> +

We have this check in many places. Should either convert all or none.

>  		return 0;
> +	}
>  
>  	switch (format) {
>  	case DRM_FORMAT_YUYV:
> -- 
> 2.5.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion

[Imre Deak]

On Thu, 2016-05-12 at 16:10 +0300, Ville Syrjälä wrote:
> On Thu, May 12, 2016 at 04:00:38PM +0300, Imre Deak wrote:
> > Returning a 0 bpp/cpp value from these functions isn't ever valid.
> > In
> > many cases it can also lead to a div-by-zero possibly at some later
> > point in time, so make sure we catch such errors as soon as
> > possible via
> > louder error reporting.
> > 
> > CC: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c
> > b/drivers/gpu/drm/drm_crtc.c
> > index 70f9c68..3a32606 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -5610,8 +5610,8 @@ void drm_fb_get_bpp_depth(uint32_t format,
> > unsigned int *depth,
> >  		*bpp = 32;
> >  		break;
> >  	default:
> > -		DRM_DEBUG_KMS("unsupported pixel format %s\n",
> > -			      drm_get_format_name(format));
> > +		WARN(1, "unsupported pixel format %s\n",
> > +		     drm_get_format_name(format));
> 
> NAK. This will happen every time drm_helper_mode_fill_fb_struct()
> is called with a non-RGB format.

Yep, missed that. So how about handling here those formats explicitly,
and emitting a warning only for truly unsupported formats?

> >  		*depth = 0;
> >  		*bpp = 0;
> >  		break;
> > @@ -5666,8 +5666,12 @@ int drm_format_plane_cpp(uint32_t format,
> > int plane)
> >  	unsigned int depth;
> >  	int bpp;
> >  
> > -	if (plane >= drm_format_num_planes(format))
> > +	if (plane >= drm_format_num_planes(format)) {
> > +		WARN(1, "invalid plane %d for format %s\n",
> > +		     plane, drm_get_format_name(format));
> > +
> 
> We have this check in many places. Should either convert all or none.

Ok, I can also change drm_format_plane_width()
and drm_format_plane_height(). Couldn't spot any other places.

> >  		return 0;
> > +	}
> >  
> >  	switch (format) {
> >  	case DRM_FORMAT_YUYV:
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion

[Ville Syrjälä]

On Thu, May 12, 2016 at 04:43:05PM +0300, Imre Deak wrote:
> On Thu, 2016-05-12 at 16:10 +0300, Ville Syrjälä wrote:
> > On Thu, May 12, 2016 at 04:00:38PM +0300, Imre Deak wrote:
> > > Returning a 0 bpp/cpp value from these functions isn't ever valid.
> > > In
> > > many cases it can also lead to a div-by-zero possibly at some later
> > > point in time, so make sure we catch such errors as soon as
> > > possible via
> > > louder error reporting.
> > > 
> > > CC: Dave Airlie <airlied@redhat.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_crtc.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_crtc.c
> > > b/drivers/gpu/drm/drm_crtc.c
> > > index 70f9c68..3a32606 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -5610,8 +5610,8 @@ void drm_fb_get_bpp_depth(uint32_t format,
> > > unsigned int *depth,
> > >  		*bpp = 32;
> > >  		break;
> > >  	default:
> > > -		DRM_DEBUG_KMS("unsupported pixel format %s\n",
> > > -			      drm_get_format_name(format));
> > > +		WARN(1, "unsupported pixel format %s\n",
> > > +		     drm_get_format_name(format));
> > 
> > NAK. This will happen every time drm_helper_mode_fill_fb_struct()
> > is called with a non-RGB format.
> 
> Yep, missed that. So how about handling here those formats explicitly,
> and emitting a warning only for truly unsupported formats?

More work to keep this list updated, and it wouldn't prevent any
div-by-zero with those formats. So I don't really see a point in that.

> 
> > >  		*depth = 0;
> > >  		*bpp = 0;
> > >  		break;
> > > @@ -5666,8 +5666,12 @@ int drm_format_plane_cpp(uint32_t format,
> > > int plane)
> > >  	unsigned int depth;
> > >  	int bpp;
> > >  
> > > -	if (plane >= drm_format_num_planes(format))
> > > +	if (plane >= drm_format_num_planes(format)) {
> > > +		WARN(1, "invalid plane %d for format %s\n",
> > > +		     plane, drm_get_format_name(format));
> > > +
> > 
> > We have this check in many places. Should either convert all or none.
> 
> Ok, I can also change drm_format_plane_width()
> and drm_format_plane_height(). Couldn't spot any other places.

I thought we might have more. I guess not.

> 
> > >  		return 0;
> > > +	}
> > >  
> > >  	switch (format) {
> > >  	case DRM_FORMAT_YUYV:
> > > -- 
> > > 2.5.0
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion

[Imre Deak]

On Thu, 2016-05-12 at 16:52 +0300, Ville Syrjälä wrote:
> On Thu, May 12, 2016 at 04:43:05PM +0300, Imre Deak wrote:
> > On Thu, 2016-05-12 at 16:10 +0300, Ville Syrjälä wrote:
> > > On Thu, May 12, 2016 at 04:00:38PM +0300, Imre Deak wrote:
> > > > Returning a 0 bpp/cpp value from these functions isn't ever
> > > > valid.
> > > > In
> > > > many cases it can also lead to a div-by-zero possibly at some
> > > > later
> > > > point in time, so make sure we catch such errors as soon as
> > > > possible via
> > > > louder error reporting.
> > > > 
> > > > CC: Dave Airlie <airlied@redhat.com>
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_crtc.c | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_crtc.c
> > > > b/drivers/gpu/drm/drm_crtc.c
> > > > index 70f9c68..3a32606 100644
> > > > --- a/drivers/gpu/drm/drm_crtc.c
> > > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > > @@ -5610,8 +5610,8 @@ void drm_fb_get_bpp_depth(uint32_t
> > > > format,
> > > > unsigned int *depth,
> > > >  		*bpp = 32;
> > > >  		break;
> > > >  	default:
> > > > -		DRM_DEBUG_KMS("unsupported pixel format %s\n",
> > > > -			      drm_get_format_name(format));
> > > > +		WARN(1, "unsupported pixel format %s\n",
> > > > +		     drm_get_format_name(format));
> > > 
> > > NAK. This will happen every time drm_helper_mode_fill_fb_struct()
> > > is called with a non-RGB format.
> > 
> > Yep, missed that. So how about handling here those formats
> > explicitly,
> > and emitting a warning only for truly unsupported formats?
> 
> More work to keep this list updated, and it wouldn't prevent any
> div-by-zero with those formats. So I don't really see a point in
> that.

It would avoid the incorrect 'unsupported pixel format' message for
those.

> > 
> > > >  		*depth = 0;
> > > >  		*bpp = 0;
> > > >  		break;
> > > > @@ -5666,8 +5666,12 @@ int drm_format_plane_cpp(uint32_t
> > > > format,
> > > > int plane)
> > > >  	unsigned int depth;
> > > >  	int bpp;
> > > >  
> > > > -	if (plane >= drm_format_num_planes(format))
> > > > +	if (plane >= drm_format_num_planes(format)) {
> > > > +		WARN(1, "invalid plane %d for format %s\n",
> > > > +		     plane, drm_get_format_name(format));
> > > > +
> > > 
> > > We have this check in many places. Should either convert all or
> > > none.
> > 
> > Ok, I can also change drm_format_plane_width()
> > and drm_format_plane_height(). Couldn't spot any other places.
> 
> I thought we might have more. I guess not.
> 
> > 
> > > >  		return 0;
> > > > +	}
> > > >  
> > > >  	switch (format) {
> > > >  	case DRM_FORMAT_YUYV:
> > > > -- 
> > > > 2.5.0
> > > > 
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion

[Daniel Vetter]

On Thu, May 12, 2016 at 05:00:06PM +0300, Imre Deak wrote:
> On Thu, 2016-05-12 at 16:52 +0300, Ville Syrjälä wrote:
> > On Thu, May 12, 2016 at 04:43:05PM +0300, Imre Deak wrote:
> > > On Thu, 2016-05-12 at 16:10 +0300, Ville Syrjälä wrote:
> > > > On Thu, May 12, 2016 at 04:00:38PM +0300, Imre Deak wrote:
> > > > > Returning a 0 bpp/cpp value from these functions isn't ever
> > > > > valid.
> > > > > In
> > > > > many cases it can also lead to a div-by-zero possibly at some
> > > > > later
> > > > > point in time, so make sure we catch such errors as soon as
> > > > > possible via
> > > > > louder error reporting.
> > > > > 
> > > > > CC: Dave Airlie <airlied@redhat.com>
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_crtc.c | 10 +++++++---
> > > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_crtc.c
> > > > > b/drivers/gpu/drm/drm_crtc.c
> > > > > index 70f9c68..3a32606 100644
> > > > > --- a/drivers/gpu/drm/drm_crtc.c
> > > > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > > > @@ -5610,8 +5610,8 @@ void drm_fb_get_bpp_depth(uint32_t
> > > > > format,
> > > > > unsigned int *depth,
> > > > >  		*bpp = 32;
> > > > >  		break;
> > > > >  	default:
> > > > > -		DRM_DEBUG_KMS("unsupported pixel format %s\n",
> > > > > -			      drm_get_format_name(format));
> > > > > +		WARN(1, "unsupported pixel format %s\n",
> > > > > +		     drm_get_format_name(format));
> > > > 
> > > > NAK. This will happen every time drm_helper_mode_fill_fb_struct()
> > > > is called with a non-RGB format.
> > > 
> > > Yep, missed that. So how about handling here those formats
> > > explicitly,
> > > and emitting a warning only for truly unsupported formats?
> > 
> > More work to keep this list updated, and it wouldn't prevent any
> > div-by-zero with those formats. So I don't really see a point in
> > that.
> 
> It would avoid the incorrect 'unsupported pixel format' message for
> those.

If that's the entire concern, delete it. New drivers shouldn't use these
functions any more anyway.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/3] drm: Tune up error message for incorrect plane/format combinations

[Imre Deak]

Returning 0 from these functions isn't ever valid. In many cases it can
also lead to a div-by-zero possibly at some later point in time, so make
sure we catch such errors as soon as possible via louder error
reporting.

v2:
- Print the same WARN whenever we check for the same condition (Ville)
- Don't change drm_fb_get_bpp_depth(), for non-RGB formats we return
  bpp=0, depth=0 normally. (Ville, Daniel)

CC: Dave Airlie <airlied@redhat.com>
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/drm_crtc.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 70f9c68..990a9de 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5544,6 +5544,13 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
 	return dev->driver->dumb_destroy(file_priv, dev, args->handle);
 }
 
+static bool check_format_plane_valid(uint32_t format, int plane)
+{
+	return !WARN(plane >= drm_format_num_planes(format),
+		     "invalid plane %d for format %s\n",
+		     plane, drm_get_format_name(format));
+}
+
 /**
  * drm_fb_get_bpp_depth - get the bpp/depth values for format
  * @format: pixel format (DRM_FORMAT_*)
@@ -5666,7 +5673,7 @@ int drm_format_plane_cpp(uint32_t format, int plane)
 	unsigned int depth;
 	int bpp;
 
-	if (plane >= drm_format_num_planes(format))
+	if (!check_format_plane_valid(format, plane))
 		return 0;
 
 	switch (format) {
@@ -5771,7 +5778,7 @@ EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
  */
 int drm_format_plane_width(int width, uint32_t format, int plane)
 {
-	if (plane >= drm_format_num_planes(format))
+	if (!check_format_plane_valid(format, plane))
 		return 0;
 
 	if (plane == 0)
@@ -5792,7 +5799,7 @@ EXPORT_SYMBOL(drm_format_plane_width);
  */
 int drm_format_plane_height(int height, uint32_t format, int plane)
 {
-	if (plane >= drm_format_num_planes(format))
+	if (!check_format_plane_valid(format, plane))
 		return 0;
 
 	if (plane == 0)
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [2/3] drm/mst: Fix error handling during MST sideband message reception

[Lyude Paul]

On Thu, 2016-05-12 at 16:00 +0300, Imre Deak wrote:
> Handle any error due to partial reads, timeouts etc. to avoid parsing
> uninitialized data subsequently. Also bail out if the parsing itself
> fails.
> 
> CC: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index a13edf5..12c0ab3 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2204,11 +2204,19 @@ static void drm_dp_get_one_sb_msg(struct
> drm_dp_mst_topology_mgr *mgr, bool up)
>  		ret = drm_dp_dpcd_read(mgr->aux, basereg + curreply,
>  				    replyblock, len);
>  		if (ret != len) {
> -			DRM_DEBUG_KMS("failed to read a chunk\n");
> +			DRM_DEBUG_KMS("failed to read a chunk (len %d, ret
> %d)\n",
> +				      len, ret);
> +
I'd get rid of the whitespace here…
> +			return;
>  		}
> +
>  		ret = drm_dp_sideband_msg_build(msg, replyblock, len, false);
> -		if (ret == false)
> +		if (!ret) {
>  			DRM_DEBUG_KMS("failed to build sideband msg\n");
> +
And here, to match the rest of the file. Otherwise looks good to me;

Reviewed-by: Lyude <cpaul@redhat.com>
> +			return;
> +		}
> +
>  		curreply += len;
>  		replylen -= len;
>  	}
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 2/3] drm/mst: Fix error handling during MST sideband message reception

[Imre Deak]

Handle any error due to partial reads, timeouts etc. to avoid parsing
uninitialized data subsequently. Also bail out if the parsing itself
fails.

v2:
- Remove blank lines before returns to match the rest of file (Lyude)

CC: Dave Airlie <airlied@redhat.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index a13edf5..9e64493 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2204,11 +2204,17 @@ static void drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
 		ret = drm_dp_dpcd_read(mgr->aux, basereg + curreply,
 				    replyblock, len);
 		if (ret != len) {
-			DRM_DEBUG_KMS("failed to read a chunk\n");
+			DRM_DEBUG_KMS("failed to read a chunk (len %d, ret %d)\n",
+				      len, ret);
+			return;
 		}
+
 		ret = drm_dp_sideband_msg_build(msg, replyblock, len, false);
-		if (ret == false)
+		if (!ret) {
 			DRM_DEBUG_KMS("failed to build sideband msg\n");
+			return;
+		}
+
 		curreply += len;
 		replylen -= len;
 	}
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req()

[Imre Deak]

On Thu, May 12, 2016 at 04:00:40PM +0300, Imre Deak wrote:
> In case of an unknown broadcast message is sent mstb will remain unset,
> so check for this.
> 
> CC: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Could someone review 2/3 and 3/3 in this patchset? 1/3 was NAK'd and
otherwise isn't needed anymore.

Thanks,
Imre

> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 12c0ab3..412b9ca 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -2334,7 +2334,9 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
>  			DRM_DEBUG_KMS("Got RSN: pn: %d avail_pbn %d\n", msg.u.resource_stat.port_number, msg.u.resource_stat.available_pbn);
>  		}
>  
> -		drm_dp_put_mst_branch_device(mstb);
> +		if (mstb)
> +			drm_dp_put_mst_branch_device(mstb);
> +
>  		memset(&mgr->up_req_recv, 0, sizeof(struct drm_dp_sideband_msg_rx));
>  	}
>  	return ret;
> -- 
> 2.5.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req()

[Lyude Paul]

Looks good to me.

Reviewed-by: Lyude <lyude@redhat.com>

On Mon, 2017-07-17 at 15:10 +0300, Imre Deak wrote:
> On Thu, May 12, 2016 at 04:00:40PM +0300, Imre Deak wrote:
> > In case of an unknown broadcast message is sent mstb will remain
> > unset,
> > so check for this.
> > 
> > CC: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> Could someone review 2/3 and 3/3 in this patchset? 1/3 was NAK'd and
> otherwise isn't needed anymore.
> 
> Thanks,
> Imre
> 
> > ---
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 12c0ab3..412b9ca 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -2334,7 +2334,9 @@ static int drm_dp_mst_handle_up_req(struct
> > drm_dp_mst_topology_mgr *mgr)
> >  			DRM_DEBUG_KMS("Got RSN: pn: %d avail_pbn
> > %d\n", msg.u.resource_stat.port_number,
> > msg.u.resource_stat.available_pbn);
> >  		}
> >  
> > -		drm_dp_put_mst_branch_device(mstb);
> > +		if (mstb)
> > +			drm_dp_put_mst_branch_device(mstb);
> > +
> >  		memset(&mgr->up_req_recv, 0, sizeof(struct
> > drm_dp_sideband_msg_rx));
> >  	}
> >  	return ret;
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel