On 2/9/2018 7:17 AM, Jani Nikula wrote:
On Thu, 08 Feb 2018, Joe Moriarty <joe.moria...@oracle.com> wrote:
The Parfait (version 2.1.0) static code analysis tool found multiple NULL
pointer derefernce problems.

Thanks for the patch. Multiple problems requires multiple patches to fix
them, one patch per problem. Please split up the patch.

Thanks,
Jani.


- drivers/gpu/drm/drm_dp_mst_topology.c
The call to drm_dp_calculate_rad() in function drm_dp_port_setup_pdt()
could result in a NULL pointer being returned to port->mstb due to a
failure to allocate memory for port->mstb.

- drivers/gpu/drm/drm_drv.c
Any calls to drm_minor_get_slot() could result in the return of a NULL
pointer when an invalid DRM device type is encountered.  2 helper
functions where added for pointer manipulation (drm_minor_get_slot()
and drm_minor_set_minor()) along with checks for valid pointers for
struct drm_device variables throughout this module.

- drivers/gpu/drm/drm_edid.c
The call to drm_cvt_mode() in function drm_mode_std() for the
HDTV hack resulted in the possibility of accessing a NULL pointer
if drm_mode_std() returned NULL.  A check for this added right after
the call to drm_cvt_mode() in this particular area of code.

- drivers/gpu/drm/drm_vblank.c
Null pointer checks were added to return values from calls to
drm_crtc_from_index().  There is a possibility, however minute, that
crtc->index may not be found when trying to find the struct crtc
from it's assigned index given in drm_crtc_init_with_planes().
3 return checks for NULL where added.

Signed-off-by: Joe Moriarty <joe.moria...@oracle.com>
Reviewed-by: Steven Sistare <steven.sist...@oracle.com>
---
  drivers/gpu/drm/drm_dp_mst_topology.c |  8 +++++---
  drivers/gpu/drm/drm_drv.c             | 38 +++++++++++++++++++++++++++++++----
  drivers/gpu/drm/drm_edid.c            |  2 ++
  drivers/gpu/drm/drm_vblank.c          |  6 +++---
  4 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 70dcfa58d3c2..ec503d416062 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1082,10 +1082,12 @@ static bool drm_dp_port_setup_pdt(struct 
drm_dp_mst_port *port)
                lct = drm_dp_calculate_rad(port, rad);
port->mstb = drm_dp_add_mst_branch_device(lct, rad);
-               port->mstb->mgr = port->mgr;
-               port->mstb->port_parent = port;
+               if (port->mstb) {
+                       port->mstb->mgr = port->mgr;
+                       port->mstb->port_parent = port;
- send_link = true;
+                       send_link = true;
+               }
                break;
        }
        return send_link;
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9acc1e157813..938eee77f014 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -99,10 +99,36 @@ static struct drm_minor **drm_minor_get_slot(struct 
drm_device *dev,
        case DRM_MINOR_CONTROL:
                return &dev->control;
        default:
+               DRM_ERROR("Error in %s: Invalid dev, type = %d\n",
+                         __func__, type);
                return NULL;
        }
  }
+static inline int drm_minor_set_minor(struct drm_device *dev,
+                                     unsigned int type,
+                                     struct drm_minor *minor)
+{
+       struct drm_minor **slot = drm_minor_get_slot(dev, type);
+       int retval = -ENODEV;
+
+       if (slot) {
+               retval = 0;
+               *slot = minor;
+       }
+       return retval;
+}
+
+static inline struct drm_minor *drm_minor_get_minor(struct drm_device *dev,
+                                                   unsigned int type)
+{
+       struct drm_minor **slot = drm_minor_get_slot(dev, type);
+
+       if (slot)
+               return *slot;
+       return NULL;
+}
+
  static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
  {
        struct drm_minor *minor;
@@ -137,8 +163,9 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned 
int type)
                goto err_index;
        }
- *drm_minor_get_slot(dev, type) = minor;
-       return 0;
+       r = drm_minor_set_minor(dev, type, minor);
+       if (r == 0)
+               return r;
err_index:
        spin_lock_irqsave(&drm_minor_lock, flags);
@@ -155,6 +182,9 @@ static void drm_minor_free(struct drm_device *dev, unsigned 
int type)
        unsigned long flags;
slot = drm_minor_get_slot(dev, type);
+       if (!slot)
+               return
+
        minor = *slot;
        if (!minor)
                return;
@@ -177,7 +207,7 @@ static int drm_minor_register(struct drm_device *dev, 
unsigned int type)
DRM_DEBUG("\n"); - minor = *drm_minor_get_slot(dev, type);
+       minor = drm_minor_get_slot(dev, type);
        if (!minor)
                return 0;
@@ -209,7 +239,7 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
        struct drm_minor *minor;
        unsigned long flags;
- minor = *drm_minor_get_slot(dev, type);
+       minor = drm_minor_get_slot(dev, type);
        if (!minor || !device_is_registered(minor->kdev))
                return;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ddd537914575..23c9977d8999 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2083,6 +2083,8 @@ drm_mode_std(struct drm_connector *connector, struct edid 
*edid,
        if (hsize == 1366 && vsize == 768 && vrefresh_rate == 60) {
                mode = drm_cvt_mode(dev, 1366, 768, vrefresh_rate, 0, 0,
                                    false);
+               if (!mode)
+                       return NULL;
                mode->hdisplay = 1366;
                mode->hsync_start = mode->hsync_start - 1;
                mode->hsync_end = mode->hsync_end - 1;
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 32d9bcf5be7f..a3a1bce87468 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -120,7 +120,7 @@ static u32 __get_vblank_counter(struct drm_device *dev, 
unsigned int pipe)
        if (drm_core_check_feature(dev, DRIVER_MODESET)) {
                struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
- if (crtc->funcs->get_vblank_counter)
+               if (crtc && crtc->funcs->get_vblank_counter)
                        return crtc->funcs->get_vblank_counter(crtc);
        }
@@ -318,7 +318,7 @@ static void __disable_vblank(struct drm_device *dev, unsigned int pipe)
        if (drm_core_check_feature(dev, DRIVER_MODESET)) {
                struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
- if (crtc->funcs->disable_vblank) {
+               if (crtc && crtc->funcs->disable_vblank) {
                        crtc->funcs->disable_vblank(crtc);
                        return;
                }
@@ -918,7 +918,7 @@ static int __enable_vblank(struct drm_device *dev, unsigned 
int pipe)
        if (drm_core_check_feature(dev, DRIVER_MODESET)) {
                struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
- if (crtc->funcs->enable_vblank)
+               if (crtc && crtc->funcs->enable_vblank)
                        return crtc->funcs->enable_vblank(crtc);
        }


Thanks for the time to review Jani. I will split the patch up on the next version.

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

Reply via email to