[PATCH] dma-buf: Precheck for a valid dma_fence before acquiring the reference

[Daniel Vetter]

On Fri, Feb 21, 2020 at 3:38 PM Chris Wilson <chris at chris-wilson.co.uk> wrote:
> dma_fence_get_rcu() is used to acquire a reference to under a dma-fence
> under racey conditions -- a perfect recipe for a disaster. As we know
> the caller may be handling stale memory, use kasan to confirm the
> dma-fence, or rather its memory block, is valid before attempting to
> acquire a reference. This should help us to more quickly and clearly
> identify lost races.

Hm ... I'm a bit lost on the purpose, and what this does. Fences need
to be rcu-freed, and I have honestly no idea how kasan treats those.
Are we throwing false positives, because kasan thinks the stuff is
freed, but we're still accessing it (while the grace period hasn't
passed, so anything freed is still guaranteed to be at least in the
slab cache somewhere).

I'm not seeing how this catches lost races quicker, since the refcount
should get to 0 way before we get to the kfree. So the refcount check
on the next line should catch strictly more races than the kasan
check.
-Daniel

> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  include/linux/dma-fence.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 3347c54f3a87..2805edd74738 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -301,6 +301,9 @@ static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
>   */
>  static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
>  {
> +       if (unlikely(!kasan_check_read(fence, sizeof(*fence))))
> +               return NULL;
> +
>         if (kref_get_unless_zero(&fence->refcount))
>                 return fence;
>         else
> --
> 2.25.1
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch