* [PATCH 0/3] drm: Fix/annotate better a few error paths
@ 2016-05-12 13:00 Imre Deak
2016-05-12 13:00 ` [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion Imre Deak
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Imre Deak @ 2016-05-12 13:00 UTC (permalink / raw)
To: dri-devel
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
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion 2016-05-12 13:00 [PATCH 0/3] drm: Fix/annotate better a few error paths Imre Deak @ 2016-05-12 13:00 ` Imre Deak 2016-05-12 13:10 ` Ville Syrjälä 2016-05-12 15:28 ` [PATCH v2 1/3] drm: Tune up error message for incorrect plane/format combinations Imre Deak 2016-05-12 13:00 ` [PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception Imre Deak 2016-05-12 13:00 ` [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() Imre Deak 2 siblings, 2 replies; 14+ messages in thread From: Imre Deak @ 2016-05-12 13:00 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion 2016-05-12 13:00 ` [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion Imre Deak @ 2016-05-12 13:10 ` Ville Syrjälä 2016-05-12 13:43 ` Imre Deak 2016-05-12 15:28 ` [PATCH v2 1/3] drm: Tune up error message for incorrect plane/format combinations Imre Deak 1 sibling, 1 reply; 14+ messages in thread From: Ville Syrjälä @ 2016-05-12 13:10 UTC (permalink / raw) To: Imre Deak; +Cc: Dave Airlie, dri-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion 2016-05-12 13:10 ` Ville Syrjälä @ 2016-05-12 13:43 ` Imre Deak 2016-05-12 13:52 ` Ville Syrjälä 0 siblings, 1 reply; 14+ messages in thread From: Imre Deak @ 2016-05-12 13:43 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Dave Airlie, dri-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion 2016-05-12 13:43 ` Imre Deak @ 2016-05-12 13:52 ` Ville Syrjälä 2016-05-12 14:00 ` Imre Deak 0 siblings, 1 reply; 14+ messages in thread From: Ville Syrjälä @ 2016-05-12 13:52 UTC (permalink / raw) To: Imre Deak; +Cc: Dave Airlie, dri-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion 2016-05-12 13:52 ` Ville Syrjälä @ 2016-05-12 14:00 ` Imre Deak 2016-05-12 14:02 ` Daniel Vetter 0 siblings, 1 reply; 14+ messages in thread From: Imre Deak @ 2016-05-12 14:00 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Dave Airlie, dri-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion 2016-05-12 14:00 ` Imre Deak @ 2016-05-12 14:02 ` Daniel Vetter 0 siblings, 0 replies; 14+ messages in thread From: Daniel Vetter @ 2016-05-12 14:02 UTC (permalink / raw) To: Imre Deak; +Cc: Dave Airlie, dri-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/3] drm: Tune up error message for incorrect plane/format combinations 2016-05-12 13:00 ` [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion Imre Deak 2016-05-12 13:10 ` Ville Syrjälä @ 2016-05-12 15:28 ` Imre Deak 1 sibling, 0 replies; 14+ messages in thread From: Imre Deak @ 2016-05-12 15:28 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Daniel Vetter 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception 2016-05-12 13:00 [PATCH 0/3] drm: Fix/annotate better a few error paths Imre Deak 2016-05-12 13:00 ` [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion Imre Deak @ 2016-05-12 13:00 ` Imre Deak 2016-05-12 22:00 ` [2/3] " Lyude Paul 2016-05-13 14:31 ` [PATCH v2 2/3] " Imre Deak 2016-05-12 13:00 ` [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() Imre Deak 2 siblings, 2 replies; 14+ messages in thread From: Imre Deak @ 2016-05-12 13:00 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [2/3] drm/mst: Fix error handling during MST sideband message reception 2016-05-12 13:00 ` [PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception Imre Deak @ 2016-05-12 22:00 ` Lyude Paul 2016-05-13 14:31 ` [PATCH v2 2/3] " Imre Deak 1 sibling, 0 replies; 14+ messages in thread From: Lyude Paul @ 2016-05-12 22:00 UTC (permalink / raw) To: Imre Deak, dri-devel; +Cc: Dave Airlie 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] drm/mst: Fix error handling during MST sideband message reception 2016-05-12 13:00 ` [PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception Imre Deak 2016-05-12 22:00 ` [2/3] " Lyude Paul @ 2016-05-13 14:31 ` Imre Deak 1 sibling, 0 replies; 14+ messages in thread From: Imre Deak @ 2016-05-13 14:31 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie, Lyude 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() 2016-05-12 13:00 [PATCH 0/3] drm: Fix/annotate better a few error paths Imre Deak 2016-05-12 13:00 ` [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion Imre Deak 2016-05-12 13:00 ` [PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception Imre Deak @ 2016-05-12 13:00 ` Imre Deak 2017-07-17 12:10 ` Imre Deak 2 siblings, 1 reply; 14+ messages in thread From: Imre Deak @ 2016-05-12 13:00 UTC (permalink / raw) To: dri-devel; +Cc: Dave Airlie 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() 2016-05-12 13:00 ` [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() Imre Deak @ 2017-07-17 12:10 ` Imre Deak 2017-07-17 15:33 ` Lyude Paul 0 siblings, 1 reply; 14+ messages in thread From: Imre Deak @ 2017-07-17 12:10 UTC (permalink / raw) To: dri-devel, Lyude Paul; +Cc: Dave Airlie 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() 2017-07-17 12:10 ` Imre Deak @ 2017-07-17 15:33 ` Lyude Paul 0 siblings, 0 replies; 14+ messages in thread From: Lyude Paul @ 2017-07-17 15:33 UTC (permalink / raw) To: imre.deak, dri-devel; +Cc: Dave Airlie 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-07-17 15:33 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-12 13:00 [PATCH 0/3] drm: Fix/annotate better a few error paths Imre Deak 2016-05-12 13:00 ` [PATCH 1/3] drm: Tune up error message during format->bpp/cpp conversion Imre Deak 2016-05-12 13:10 ` Ville Syrjälä 2016-05-12 13:43 ` Imre Deak 2016-05-12 13:52 ` Ville Syrjälä 2016-05-12 14:00 ` Imre Deak 2016-05-12 14:02 ` Daniel Vetter 2016-05-12 15:28 ` [PATCH v2 1/3] drm: Tune up error message for incorrect plane/format combinations Imre Deak 2016-05-12 13:00 ` [PATCH 2/3] drm/mst: Fix error handling during MST sideband message reception Imre Deak 2016-05-12 22:00 ` [2/3] " Lyude Paul 2016-05-13 14:31 ` [PATCH v2 2/3] " Imre Deak 2016-05-12 13:00 ` [PATCH 3/3] drm: Avoid dereferencing a NULL mstb in drm_dp_mst_handle_up_req() Imre Deak 2017-07-17 12:10 ` Imre Deak 2017-07-17 15:33 ` Lyude Paul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).