[bug report] dma-buf: Move dma_buf_attach() to dynamic locking specification

[Dmitry Osipenko]

On 10/25/22 14:41, Dan Carpenter wrote:
> Hello Dmitry Osipenko,
> 
> The patch 809d9c72c2f8: "dma-buf: Move dma_buf_attach() to dynamic
> locking specification" from Oct 17, 2022, leads to the following
> Smatch static checker warning:
> 
> 	drivers/dma-buf/dma-buf.c:957 dma_buf_dynamic_attach()
> 	error: double unlocked 'dmabuf->resv' (orig line 915)
> 
> drivers/dma-buf/dma-buf.c
>    987  /**
>    988   * dma_buf_detach - Remove the given attachment from dmabuf's attachments list
>    989   * @dmabuf:     [in]    buffer to detach from.
>    990   * @attach:     [in]    attachment to be detached; is free'd after this call.
>    991   *
>    992   * Clean up a device attachment obtained by calling dma_buf_attach().
>    993   *
>    994   * Optionally this calls &dma_buf_ops.detach for device-specific detach.
>    995   */
>    996  void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
>    997  {
>    998          if (WARN_ON(!dmabuf || !attach))
>    999                  return;
>   1000  
>   1001          dma_resv_lock(attach->dmabuf->resv, NULL);
> 
> In the original code used to take this both the "attach->dmabuf->resv"
> and "dmabuf->resv" locks and unlock them both.  But now it takes one
> lock and unlocks the other.  Seems sus.

It will be the same lock. Apparently I copied the part of code from
other function, that's why lock/unlock aren't consistent there. The
dma_buf_detach() doesn't really need the `dmabuf` argument, perhaps it
was more useful in the past.

I'll prepare the patch to clean up the locking pointer. Thank you for
the report!

-- 
Best regards,
Dmitry