• Daniel Vetter's avatar
    drm/gem: fix up flink name create race · a8e11d1c
    Daniel Vetter authored
    This is the 2nd attempt, I've always been a bit dissatisified with the
    tricky nature of the first one:
    
    http://lists.freedesktop.org/archives/dri-devel/2012-July/025451.html
    
    The issue is that the flink ioctl can race with calling gem_close on
    the last gem handle. In that case we'll end up with a zero handle
    count, but an flink name (and it's corresponding reference). Which
    results in a neat space leak.
    
    In my first attempt I've solved this by rechecking the handle count.
    But fundamentally the issue is that ->handle_count isn't your usual
    refcount - it can be resurrected from 0 among other things.
    
    For those special beasts atomic_t often suggest way more ordering that
    it actually guarantees. To prevent being tricked by those hairy
    semantics take the easy way out and simply protect the handle with the
    existing dev->object_name_lock.
    
    With that change implemented it's dead easy to fix the flink vs. gem
    close reace: When we try to create the name we simply have to check
    whether there's still officially a gem handle around and if not refuse
    to create the flink name. Since the handle count decrement and flink
    name destruction is now also protected by that lock the reace is gone
    and we can't ever leak the flink reference again.
    
    Outside of the drm core only the exynos driver looks at the handle
    count, and tbh I have no idea why (it's just for debug dmesg output
    luckily).
    
    I've considered inlining the drm_gem_object_handle_free, but I plan to
    add more name-like things (like the exported dma_buf) to this scheme,
    so it's clearer to leave the handle freeing in its own function.
    
    This is exercised by the new gem_flink_race i-g-t testcase, which on
    my snb leaks gem objects at a rate of roughly 1k objects/s.
    
    v2: Fix up the error path handling in handle_create and make it more
    robust by simply calling object_handle_unreference.
    
    v3: Fix up the handle_unreference logic bug - atomic_dec_and_test
    retursn 1 for 0. Oops.
    
    v4: Squash in inlining of drm_gem_object_handle_reference as suggested
    by Dave Airlie and add a note that we now have a testcase.
    
    Cc: Dave Airlie <airlied@gmail.com>
    Cc: Inki Dae <inki.dae@samsung.com>
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
    a8e11d1c
drm_gem.c 22.5 KB