[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <da261a4c-6c27-454f-b21d-af1814b58b91@wanadoo.fr>
Date: Sat, 5 Apr 2025 18:17:08 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: 18255117159@....com
Cc: bhelgaas@...gle.com, jonathanh@...dia.com, kw@...ux.com,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-tegra@...r.kernel.org, lpieralisi@...nel.org,
manivannan.sadhasivam@...aro.org, robh@...nel.org, thierry.reding@...il.com
Subject: Re: [PATCH] pci: tegra194: Fix debugfs cleanup for !CONFIG_PCIEASPM
Le 05/04/2025 à 17:49, Hans Zhang a écrit :
>
>
> 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
debugfs IS initialized, because it is in a structure allocated with
devm_kzalloc().
And debugfs functions handle such cases.
CJ
> 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
>
>
>
Powered by blists - more mailing lists