Re: [PATCH 12/25] drm/tegra: Use new drm_fb_helper functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 13, 2015 at 12:08:08PM +0530, Archit Taneja wrote:
[...]
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
[...]
> @@ -224,11 +224,11 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>  	if (IS_ERR(bo))
>  		return PTR_ERR(bo);
>  
> -	info = framebuffer_alloc(0, drm->dev);
> -	if (!info) {
> +	info = drm_fb_helper_alloc_fbi(helper);
> +	if (IS_ERR(info)) {
>  		dev_err(drm->dev, "failed to allocate framebuffer info\n");
> -		drm_gem_object_unreference_unlocked(&bo->gem);
> -		return -ENOMEM;
> +		err = PTR_ERR(info);
> +		goto gem_unref;
>  	}
>  
>  	fbdev->fb = tegra_fb_alloc(drm, &cmd, &bo, 1);
> @@ -236,7 +236,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>  		err = PTR_ERR(fbdev->fb);
>  		dev_err(drm->dev, "failed to allocate DRM framebuffer: %d\n",
>  			err);
> -		drm_gem_object_unreference_unlocked(&bo->gem);
>  		goto release;
>  	}
>  
> @@ -248,12 +247,6 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>  	info->flags = FBINFO_FLAG_DEFAULT;
>  	info->fbops = &tegra_fb_ops;
>  
> -	err = fb_alloc_cmap(&info->cmap, 256, 0);
> -	if (err < 0) {
> -		dev_err(drm->dev, "failed to allocate color map: %d\n", err);
> -		goto destroy;
> -	}
> -
>  	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->depth);
>  	drm_fb_helper_fill_var(info, helper, fb->width, fb->height);
>  
> @@ -282,7 +275,9 @@ destroy:
>  	drm_framebuffer_unregister_private(fb);
>  	tegra_fb_destroy(fb);
>  release:
> -	framebuffer_release(info);
> +	drm_fb_helper_release_fbi(helper);
> +gem_unref:
> +	drm_gem_object_unreference_unlocked(&bo->gem);

You can't do this. tegra_fb_alloc() takes ownership of the buffer, so
unless you compensate for that (by taking an explicit reference to the
buffer in tegra_fb_alloc()) you'd be dereferencing twice and likely end
up with use-after-free errors later on.

I'd prefer to keep the error clean up as it is.

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux