| Message ID | 1372673193-18824-4-git-send-email-sw0312.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 01, 2013 at 07:06:33PM +0900, Seung-Woo Kim wrote: > From: YoungJun Cho <yj44.cho@samsung.com> > > There are wrong cases to handle error in drm_open_helper(). > The priv->minor, assigned by idr_find() which can return NULL, > should be checked whether it is NULL or not before referencing it. > And if an error occurs after executing dev->driver->open() which > allocates driver specific per-file private data, then the private > data should be released. > > Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> > Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > drivers/gpu/drm/drm_fops.c | 17 +++++++++++++---- > 1 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c > index 429e07d..0470261 100644 > --- a/drivers/gpu/drm/drm_fops.c > +++ b/drivers/gpu/drm/drm_fops.c > @@ -271,6 +271,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp, > priv->uid = current_euid(); > priv->pid = get_pid(task_pid(current)); > priv->minor = idr_find(&drm_minors_idr, minor_id); > + if (!priv->minor) { > + ret = -ENOMEM; Elsewhere we use ENODEV for a failure to find the minor inode. The error path cleanup changes look reasonable. Though require a quick audit to make sure all of the callees do not expect more state to be correctly setup before being called. -Chris
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 429e07d..0470261 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -271,6 +271,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp, priv->uid = current_euid(); priv->pid = get_pid(task_pid(current)); priv->minor = idr_find(&drm_minors_idr, minor_id); + if (!priv->minor) { + ret = -ENOMEM; + goto out_free; + } + priv->ioctl_count = 0; /* for compatibility root is always authenticated */ priv->authenticated = capable(CAP_SYS_ADMIN); @@ -304,7 +309,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp, if (!priv->minor->master) { mutex_unlock(&dev->struct_mutex); ret = -ENOMEM; - goto out_free; + goto out_close; } priv->is_master = 1; @@ -322,7 +327,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp, drm_master_put(&priv->minor->master); drm_master_put(&priv->master); mutex_unlock(&dev->struct_mutex); - goto out_free; + goto out_close; } } mutex_lock(&dev->struct_mutex); @@ -333,7 +338,7 @@ static int drm_open_helper(struct inode *inode, struct file *filp, drm_master_put(&priv->minor->master); drm_master_put(&priv->master); mutex_unlock(&dev->struct_mutex); - goto out_free; + goto out_close; } } mutex_unlock(&dev->struct_mutex); @@ -367,7 +372,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp, #endif return 0; - out_free: + +out_close: + if (dev->driver->postclose) + dev->driver->postclose(dev, priv); +out_free: kfree(priv); filp->private_data = NULL; return ret;