On 24/07/2024 19:08, Dan Carpenter wrote:
> Hello Tomer Tayar,
>
> Commit 09524eb8824e ("accel/habanalabs: enforce release order of
> compute device and dma-buf") from Jan 22, 2023 (linux-next), leads to
> the following Smatch static checker warning:
>
> drivers/accel/habanalabs/common/memory.c:1844 hl_release_dmabuf()
> error: dereferencing freed memory 'ctx' (line 1841)
>
> drivers/accel/habanalabs/common/memory.c
> 1827 static void hl_release_dmabuf(struct dma_buf *dmabuf)
> 1828 {
> 1829 struct hl_dmabuf_priv *hl_dmabuf = dmabuf->priv;
> 1830 struct hl_ctx *ctx;
> 1831
> 1832 if (!hl_dmabuf)
> 1833 return;
> 1834
> 1835 ctx = hl_dmabuf->ctx;
> 1836
> 1837 if (hl_dmabuf->memhash_hnode)
> 1838 memhash_node_export_put(ctx, hl_dmabuf->memhash_hnode);
> 1839
> 1840 atomic_dec(&ctx->hdev->dmabuf_export_cnt);
> 1841 hl_ctx_put(ctx);
> ^^^
> This will free ctx on the last reference
>
> 1842
> 1843 /* Paired with get_file() in export_dmabuf() */
> --> 1844 fput(ctx->hpriv->file_priv->filp);
> ^^^
> Potential use after free
Thanks for notifying us about this warning.
Actually, because of this commit, the call to hl_ctx_put() here cannot
be last.
The release of the device file has another reference decrement [
hl_device_release() -> hl_ctx_mgr_fini() ], and this change prevents
that release as long as a dma-buf object is alive.
However, I will revise the function to get a pointer to
'ctx->hpriv->file_priv->filp' before calling hl_ctx_put(), so we won't
have the warning.
Thanks,
Tomer
>
> 1845
> 1846 kfree(hl_dmabuf);
> 1847 }
>
> regards,
> dan carpenter
