On Sat, Jul 26, 2025 at 02:41:41PM -0700, Sultan Alsawaf wrote:

quoted

On Fri, Jul 25, 2025 at 05:22:41PM +0800, Du, Bin wrote:

quoted

quoted

quoted

+		dev_warn(buf->dev, "ignore buffer free, refcount %u > 0",
+			 refcount_read(&buf->refcount));

This refcount_read() is a possible use-after-free because `buf` is accessed
after isp4vid_vb2_put() puts its reference to `buf`. So something else could put
the last reference to `buf` and free it after this refcount dec but before the
refcount_read(). Maybe just remove this dev_warn() entirely?

The warning is important to debug mem related issue, plan to keep it but
without accessing buf or buf->refcount here. Do you think it acceptible?

Yes, that sounds good. So something like this:
`dev_warn(buf->dev, "ignore buffer free, refcount > 0");`

Sorry, to fix the dev_warn() we need to make a copy of buf->dev first:

--- a/drivers/media/platform/amd/isp4/isp4_video.c
+++ b/drivers/media/platform/amd/isp4/isp4_video.c

@@ -584,8 +584,9 @@ static void isp4vid_vb2_put(void *buf_priv)
  {
  	struct isp4vid_vb2_buf *buf = (struct isp4vid_vb2_buf *)buf_priv;
  	struct amdgpu_bo *bo = (struct amdgpu_bo *)buf->bo;
+	struct device *dev = buf->dev;
  
-	dev_dbg(buf->dev,
+	dev_dbg(dev,
  		"release isp user bo 0x%llx size %ld refcount %d is_expbuf %d",
  		buf->gpu_addr, buf->size,
  		buf->refcount.refs.counter, buf->is_expbuf);

@@ -601,8 +602,7 @@ static void isp4vid_vb2_put(void *buf_priv)
  		kfree(buf);
  		buf = NULL;
  	} else {
-		dev_warn(buf->dev, "ignore buffer free, refcount %u > 0",
-			 refcount_read(&buf->refcount));
+		dev_warn(dev, "ignore buffer free, refcount > 0\n");
  	}
  }
  
--

Sultan