lkml.org 
[lkml]   [2016]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.16.y-ckt 141/180] drm/i915: On fb alloc failure, unref gem object where it gets refed
    Date
    3.16.7-ckt24 -stable review patch.  If anyone has any objections, please let me know.

    ---8<------------------------------------------------------------

    From: Lukas Wunner <lukas@wunner.de>

    commit dcb1394e74e32e33856e877bca1a0e88c78323d8 upstream.

    Currently when allocating a framebuffer fails, the gem object gets
    unrefed at the bottom of the call stack in __intel_framebuffer_create,
    not where it gets refed, which is in intel_framebuffer_create_for_mode
    (via i915_gem_alloc_object) and in intel_user_framebuffer_create
    (via drm_gem_object_lookup).

    This invites mistakes: __intel_framebuffer_create is also called from
    intelfb_alloc, and as discovered by Tvrtko Ursulin, a double unref
    was introduced there with a8bb6818270c ("drm/i915: Fix error path leak
    in fbdev fb allocation").

    As suggested by Ville Syrjälä, fix the double unref and improve code
    clarity by moving the unref away from __intel_framebuffer_create to
    where the gem object gets refed.

    Based on Tvrtko Ursulin's original v2.

    v3: On fb alloc failure, unref gem object where it gets refed,
    fix double unref in separate commit (Ville Syrjälä)

    v4: Lock struct_mutex on unref (Chris Wilson)

    v5: Rebase on drm-intel-nightly 2015y-09m-01d-09h-06m-08s UTC,
    rephrase commit message (Jani Nicula)

    Tested-by: Pierre Moreau <pierre.morrow@free.fr>
    [MBP 5,3 2009 nvidia MCP79 + G96 pre-retina]
    Tested-by: Paul Hordiienko <pvt.gord@gmail.com>
    [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina]
    Tested-by: William Brown <william@blackhats.net.au>
    [MBP 8,2 2011 intel SNB + amd turks pre-retina]
    Tested-by: Lukas Wunner <lukas@wunner.de>
    [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina]
    Tested-by: Bruno Bierbaumer <bruno@bierbaumer.net>
    [MBP 11,3 2013 intel HSW + nvidia GK107 retina]

    Fixes: a8bb6818270c ("drm/i915: Fix error path leak in fbdev fb
    allocation")
    Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
    Cc: Chris Wilson <chris@chris-wilson.co.uk>
    Signed-off-by: Lukas Wunner <lukas@wunner.de>
    Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Link: http://patchwork.freedesktop.org/patch/msgid/2161c5062ef5d6458f8ae14d924a26d4d1dba317.1446892879.git.lukas@wunner.de
    Signed-off-by: Jani Nikula <jani.nikula@intel.com>
    Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
    ---
    drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++-------
    1 file changed, 14 insertions(+), 7 deletions(-)

    diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
    index 0d68c70368ff..58563e2574f8 100644
    --- a/drivers/gpu/drm/i915/intel_display.c
    +++ b/drivers/gpu/drm/i915/intel_display.c
    @@ -8219,20 +8219,17 @@ __intel_framebuffer_create(struct drm_device *dev,
    int ret;

    intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
    - if (!intel_fb) {
    - drm_gem_object_unreference(&obj->base);
    + if (!intel_fb)
    return ERR_PTR(-ENOMEM);
    - }

    ret = intel_framebuffer_init(dev, intel_fb, mode_cmd, obj);
    if (ret)
    goto err;

    return &intel_fb->base;
    +
    err:
    - drm_gem_object_unreference(&obj->base);
    kfree(intel_fb);
    -
    return ERR_PTR(ret);
    }

    @@ -8272,6 +8269,7 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
    struct drm_display_mode *mode,
    int depth, int bpp)
    {
    + struct drm_framebuffer *fb;
    struct drm_i915_gem_object *obj;
    struct drm_mode_fb_cmd2 mode_cmd = { 0 };

    @@ -8286,7 +8284,11 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
    bpp);
    mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth);

    - return intel_framebuffer_create(dev, &mode_cmd, obj);
    + fb = intel_framebuffer_create(dev, &mode_cmd, obj);
    + if (IS_ERR(fb))
    + drm_gem_object_unreference_unlocked(&obj->base);
    +
    + return fb;
    }

    static struct drm_framebuffer *
    @@ -11421,6 +11423,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
    struct drm_file *filp,
    struct drm_mode_fb_cmd2 *mode_cmd)
    {
    + struct drm_framebuffer *fb;
    struct drm_i915_gem_object *obj;

    obj = to_intel_bo(drm_gem_object_lookup(dev, filp,
    @@ -11428,7 +11431,11 @@ intel_user_framebuffer_create(struct drm_device *dev,
    if (&obj->base == NULL)
    return ERR_PTR(-ENOENT);

    - return intel_framebuffer_create(dev, mode_cmd, obj);
    + fb = intel_framebuffer_create(dev, mode_cmd, obj);
    + if (IS_ERR(fb))
    + drm_gem_object_unreference_unlocked(&obj->base);
    +
    + return fb;
    }

    #ifndef CONFIG_DRM_I915_FBDEV
    \
     
     \ /
      Last update: 2016-02-04 00:41    [W:4.036 / U:0.608 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site