From a8bb6818270c32126dba0fd2ddb139d885c5687d Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 10 Feb 2014 18:00:39 +0100 Subject: [PATCH] drm/i915: Fix error path leak in fbdev fb allocation In Jesse's patch to switch the fbdev framebuffer from an embedded struct to a pointer the kfree in case of an error was missed. Fix this up by using our own internal fb allocation helper directly instead of reinventing that wheel. We need a to_intel_framebuffer cast unfortunately since all the other callers of _create still look better whith using a drm_framebuffer as return pointer. v2: Add an unlocked __intel_framebuffer_create function since our dev->struct_mutex locking is too much a mess. With ppgtt we even need it to take a look at the global gtt offset of pinned objects, since the vma list might chance from underneath us. At least with the current global gtt lookup functions. Reported by Mika. Cc: Mika Kuoppala Cc: Jesse Barnes Reviewed-by: Jesse Barnes Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++------- drivers/gpu/drm/i915/intel_drv.h | 4 ++-- drivers/gpu/drm/i915/intel_fbdev.c | 20 +++++++--------- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6600931f213c..6ac4c23acc77 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7690,10 +7690,15 @@ static struct drm_display_mode load_detect_mode = { 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC), }; -static struct drm_framebuffer * -intel_framebuffer_create(struct drm_device *dev, - struct drm_mode_fb_cmd2 *mode_cmd, - struct drm_i915_gem_object *obj) +static int intel_framebuffer_init(struct drm_device *dev, + struct intel_framebuffer *ifb, + struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_i915_gem_object *obj); + +struct drm_framebuffer * +__intel_framebuffer_create(struct drm_device *dev, + struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_i915_gem_object *obj) { struct intel_framebuffer *intel_fb; int ret; @@ -7704,12 +7709,7 @@ intel_framebuffer_create(struct drm_device *dev, return ERR_PTR(-ENOMEM); } - ret = i915_mutex_lock_interruptible(dev); - if (ret) - goto err; - ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj); - mutex_unlock(&dev->struct_mutex); if (ret) goto err; @@ -7721,6 +7721,23 @@ err: return ERR_PTR(ret); } +struct drm_framebuffer * +intel_framebuffer_create(struct drm_device *dev, + struct drm_mode_fb_cmd2 *mode_cmd, + struct drm_i915_gem_object *obj) +{ + struct drm_framebuffer *fb; + int ret; + + ret = i915_mutex_lock_interruptible(dev); + if (ret) + return ERR_PTR(ret); + fb = __intel_framebuffer_create(dev, mode_cmd, obj); + mutex_unlock(&dev->struct_mutex); + + return fb; +} + static u32 intel_framebuffer_pitch_for_width(int width, int bpp) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 59348a4d0238..aff9171a91d8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -681,8 +681,8 @@ int intel_pin_and_fence_fb_obj(struct drm_device *dev, struct drm_i915_gem_object *obj, struct intel_ring_buffer *pipelined); void intel_unpin_fb_obj(struct drm_i915_gem_object *obj); -int intel_framebuffer_init(struct drm_device *dev, - struct intel_framebuffer *ifb, +struct drm_framebuffer * +__intel_framebuffer_create(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_i915_gem_object *obj); void intel_prepare_page_flip(struct drm_device *dev, int plane); diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 9aa26e59a1ab..cf4627387c5a 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -62,20 +62,12 @@ static int intelfb_alloc(struct drm_fb_helper *helper, { struct intel_fbdev *ifbdev = container_of(helper, struct intel_fbdev, helper); - struct intel_framebuffer *fb; + struct drm_framebuffer *fb; struct drm_device *dev = helper->dev; struct drm_mode_fb_cmd2 mode_cmd = {}; struct drm_i915_gem_object *obj; int size, ret; - fb = kzalloc(sizeof(*fb), GFP_KERNEL); - if (!fb) { - ret = -ENOMEM; - goto out; - } - - ifbdev->fb = fb; - /* we don't do packed 24bpp */ if (sizes->surface_bpp == 24) sizes->surface_bpp = 32; @@ -102,13 +94,17 @@ static int intelfb_alloc(struct drm_fb_helper *helper, /* Flush everything out, we'll be doing GTT only from now on */ ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); if (ret) { - DRM_ERROR("failed to pin fb: %d\n", ret); + DRM_ERROR("failed to pin obj: %d\n", ret); goto out_unref; } - ret = intel_framebuffer_init(dev, ifbdev->fb, &mode_cmd, obj); - if (ret) + fb = __intel_framebuffer_create(dev, &mode_cmd, obj); + if (IS_ERR(fb)) { + ret = PTR_ERR(fb); goto out_unpin; + } + + ifbdev->fb = to_intel_framebuffer(fb); return 0; -- 2.20.1