On 07/14/2015 02:20 PM, Thierry Reding wrote:
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.

Okay. I missed out on the fact that tegra_fb_destroy in the clean up
code calls drm_gem_object_unreference_unlocked. I'll keep it as is.

Archit

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to