Received-SPF: pass (google.com: domain of linux-kernel+bounces-93992-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Feedback-ID: ie8e14432:Fastmail
Date: Wed, 6 Mar 2024 22:40:56 +0900
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Edmund Raile <edmund.raile@proton.me>
Cc: linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] firewire: ohci: prevent leak of left-over IRQ on
 unbind
Message-ID: <20240306134056.GA124318@workstation.local>
Mail-Followup-To: Edmund Raile <edmund.raile@proton.me>,
	linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
References: <20240229101236.8074-1-edmund.raile@proton.me>
 <20240229144723.13047-2-edmund.raile@proton.me>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240229144723.13047-2-edmund.raile@proton.me>

Hi,

On Thu, Feb 29, 2024 at 02:47:59PM +0000, Edmund Raile wrote:
> 
> Commit 5a95f1ded28691e6 ("firewire: ohci: use devres for requested IRQ")
> also removed the call to free_irq() in pci_remove(), leading to a
> leftover irq of devm_request_irq() at pci_disable_msi() in pci_remove()
> when unbinding the driver from the device
> 
> remove_proc_entry: removing non-empty directory 'irq/136', leaking at
> least 'firewire_ohci'
> Call Trace:
>  ? remove_proc_entry+0x19c/0x1c0
>  ? __warn+0x81/0x130
>  ? remove_proc_entry+0x19c/0x1c0
>  ? report_bug+0x171/0x1a0
>  ? console_unlock+0x78/0x120
>  ? handle_bug+0x3c/0x80
>  ? exc_invalid_op+0x17/0x70
>  ? asm_exc_invalid_op+0x1a/0x20
>  ? remove_proc_entry+0x19c/0x1c0
>  unregister_irq_proc+0xf4/0x120
>  free_desc+0x3d/0xe0
>  ? kfree+0x29f/0x2f0
>  irq_free_descs+0x47/0x70
>  msi_domain_free_locked.part.0+0x19d/0x1d0
>  msi_domain_free_irqs_all_locked+0x81/0xc0
>  pci_free_msi_irqs+0x12/0x40
>  pci_disable_msi+0x4c/0x60
>  pci_remove+0x9d/0xc0 [firewire_ohci
>      01b483699bebf9cb07a3d69df0aa2bee71db1b26]
>  pci_device_remove+0x37/0xa0
>  device_release_driver_internal+0x19f/0x200
>  unbind_store+0xa1/0xb0
> 
> remove irq with devm_free_irq() before pci_disable_msi()
> also remove it in fail_msi: of pci_probe() as this would lead to
> an identical leak
> 
> Fixes: 5a95f1ded28691e6 ("firewire: ohci: use devres for requested IRQ")
> 
> Signed-off-by: Edmund Raile <edmund.raile@proton.me>

Applied to for-linus branch. I'll send it for v6.8-final.

I think the pairs of 'pci_alloc_irq_vectors()' and 'request_irq()',
'free_irq()' and 'pci_free_irq_vectors()' would be fine here, but the
replacement of legacy API is not welcome in the last week of kernel
development, so I postpone the work to the future.

Thanks


Takashi Sakamoto