Re: [PATCH] pci: tegra194: Fix debugfs cleanup for !CONFIG_PCIEASPM

[Hans Zhang <18255117159@163.com>]

On 2025/4/5 23:28, Bjorn Helgaas wrote:
> Follow subject line capitalization convention.
> 
> On Sat, Apr 05, 2025 at 10:54:59PM +0800, Hans Zhang wrote:
>> When CONFIG_PCIEASPM is disabled, debugfs entries are not created, but
>> tegra_pcie_dw_remove() and tegra_pcie_dw_shutdown() unconditionally call
>> debugfs_remove_recursive(), leading to potential NULL pointer operations.
>>
>> Introduce deinit_debugfs() to wrap debugfs_remove_recursive(), which is
>> stubbed for !CONFIG_PCIEASPM. Use this function during removal/shutdown to
>> ensure debugfs cleanup only occurs when entries were initialized.
>>
>> This prevents kernel warnings and instability when ASPM support is
>> disabled.
> 
> This looks like there should be a Fixes: tag to connect this to the
> commit that introduced the problem.

Hi Bjorn,

Thanks your for reply. Will add.

Fixes: bb617cbd8151 (PCI: tegra194: Clean up the exit path for Endpoint 
mode)

> 
> If this is something that broke with the v6.15 merge window, we should
> include this in v6.15 via pci/for-linus.  If this broke earlier, we
> would have to decide whether pci/for-linus is still appropriate or a
> stable tag.
> 

The original code that introduced the unconditional 
`debugfs_remove_recursive()` calls was actually merged in an earlier cycle.

> We did merge some debugfs things for v6.15, but I don't see anything
> specific to pcie-tegra194.c, so I'm confused about why this fix would
> be in pcie-tegra194.c instead of some more generic place.
> 

The Tegra194 driver conditionally initializes pcie->debugfs based on 
CONFIG_PCIEASPM. When ASPM is disabled, pcie->debugfs remains 
uninitialized, but tegra_pcie_dw_remove() and tegra_pcie_dw_shutdown() 
unconditionally call debugfs_remove_recursive(), leading to a NULL 
pointer dereference. This is specific to the Tegra194 implementation, as 
other drivers or core PCI code may already guard debugfs cleanup against 
uninitialized states through different mechanisms.

Best regards,
Hans