On Mon, Jul 01, 2013 at 07:06:33PM +0900, Seung-Woo Kim wrote:
> From: YoungJun Cho <yj44.cho at 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 at samsung.com>
> Signed-off-by: Seung-Woo Kim <sw0312.kim at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at 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
--
Chris Wilson, Intel Open Source Technology Centre
