linux-kernel - Re: [PATCH v2 6/8] media: platform: amd: isp4 video node and buffers handling added

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <13296d80-082c-4e36-9c4a-b23528928186@amd.com>
Date: Tue, 29 Jul 2025 14:12:58 +0800
From: "Du, Bin" <bin.du@....com>
To: Sultan Alsawaf <sultan@...neltoast.com>
Cc: mchehab@...nel.org, hverkuil@...all.nl,
 laurent.pinchart+renesas@...asonboard.com, bryan.odonoghue@...aro.org,
 sakari.ailus@...ux.intel.com, prabhakar.mahadev-lad.rj@...renesas.com,
 linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 pratap.nirujogi@....com, benjamin.chan@....com, king.li@....com,
 gjorgji.rosikopulos@....com, Phil.Jawich@....com, Dominic.Antony@....com,
 bin.du@....com
Subject: Re: [PATCH v2 6/8] media: platform: amd: isp4 video node and buffers
 handling added

Many thanks Sultan, will try your changes in this and other mails 
together and let you know the result

On 7/27/2025 5:50 AM, Sultan Alsawaf wrote:
> On Sat, Jul 26, 2025 at 02:41:41PM -0700, Sultan Alsawaf wrote:
>> On Fri, Jul 25, 2025 at 05:22:41PM +0800, Du, Bin wrote:
>>>>> +		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