Re: Re: [BUG] firewire: core-cdev: reference count leak in iso_resource_work

Takashi Sakamoto <[email protected]> Sat, 4 Apr 2026 20:09:36 +0900
Newsgroups gmane.linux.kernel.firewire.devel
Message-ID <[email protected]>
Hi,

On Wed, Apr 01, 2026 at 05:32:39PM -0400, Dingisoul wrote:
> Hi Takashi,
> 
> Thank you for the quick reply!
> 
> You are correct that the client_put() after the 'out' label,
> releases the reference for the currently executing work.
> 
> However, the reference leak happens in the pending work that
> is canceled through cancel_delayed_work. Here is the detailed
> analysis:
> 
>     CPU 0 (Executing)                 CPU 1 (Pending)
>     -------------                     ------------
>     schedule_iso_resource()
>       client_get() // get 1
> 
>     iso_resource_work()
>                                       schedule_iso_resource()
>                                         client_get() // get 2
> 
>       cancel_delayed_work()
>       client_put() // put 1
> 
> 1. First thread calls schedule_iso_resource. It calls client_get
>    (get 1) and queues r->work.
> 
> 2. When first thread runs iso_resource_work, second thread calls 
>    schedule_iso_resource again. It calls client_get (get 2) and 
>    queues a new pending work item.
> 
> 3. The first thread calls cancel_delayed_work which removes the 
>    pending work from the queue. Because that work will never run, 
>    the corresponding client_put after the 'out' label is never called.
> 
> 4. The first thread call client_put (put 1) and finish the work.
> 
> The reference acquired in step 2 is never put, causing a reference leak.
> 
> Does this clarify the scenario? I'd be happy to provide more information.
> 
> Thanks for your time!

Now I got it. Indeed, we have the issue in current code. The work item
could be accessible in several contexts by enumerating
'struct client.resourcew_xa'. The most probable scenario is the call of
fw_cdev_update() when bus reset occurs.

Hm. As a quick glance, I can not find a quick solution for this issue.
Let me take more time to fix it (probably it requires code-refactoring).


Thanks

Takashi Sakamoto