| 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
| ||
|
Message-ID:
<PN0PR01MB10393C19522C3D75500E770B1FED02@PN0PR01MB10393.INDPRD01.PROD.OUTLOOK.COM>
Date: Thu, 13 Mar 2025 07:17:04 +0800
From: Chen Wang <unicorn_wang@...look.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: "lpieralisi@...nel.org >> Lorenzo Pieralisi" <lpieralisi@...nel.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Chen Wang <unicornxw@...il.com>,
kw@...ux.com, robh@...nel.org, s-vadapalli@...com,
thomas.richard@...tlin.com, bwawrzyn@...co.com,
wojciech.jasko-EXT@...tinental-corporation.com, kishon@...nel.org,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
sophgo@...ts.linux.dev
Subject: Re: [PATCH] PCI: cadence: Fix NULL pointer error for ops
On 2025/3/12 23:33, Bjorn Helgaas wrote:
> On Wed, Mar 12, 2025 at 10:08:43AM +0800, Chen Wang wrote:
>> Hello, Bjorn, Lorenzo & Manivannan,
>>
>> I find your names in MAINTAINERS for PCI controllers, could you please pick
>> this patch for v6.15?
>>
>> Or who else should I submit a PR for this patch to?
>>
>> BTW, Siddharth signed the review for this patch (see [1]). Please add this
>> when submitting, thanks in advance.
>>
>> Link:
>> https://lore.kernel.org/linux-pci/20250307151949.7rmxl22euubnzzpj@uda0492258/
>> [1]
>> On 2025/3/4 16:17, Chen Wang wrote:
>>> From: Chen Wang <unicorn_wang@...look.com>
>>>
>>> ops of struct cdns_pcie may be NULL, direct use
>>> will result in a null pointer error.
>>>
>>> Add checking of pcie->ops before using it.
>>>
>>> Fixes: 40d957e6f9eb ("PCI: cadence: Add support to start link and verify link status")
> AFAICT this does not fix a problem in 40d957e6f9eb, since there is no
> driver that calls cdns_pcie_host_setup() or cdns_pcie_ep_setup() with
> a NULL pcie->ops pointer, so I think you should drop this Fixes: tag.
>
> I see that you probably want to *add* an sg2042 driver [2] where you
> don't need a pcie->ops pointer (although the current patch at [2]
> *does* supply a valid pointer).
>
> So there's no urgency to apply this until you post an sg2042 driver
> that doesn't fill in the pcie->ops pointer. The best way to do this
> would be to include this patch in the series that adds the sg2042
> driver.
>
> Then the commit log can explain exactly why we need it (because the
> sg2042 in the next patch of the series doesn't need a pcie->ops
> pointer), and it will be easy to review.
>
> [2] https://lore.kernel.org/r/ddedd8f76f83fea2c6d3887132d2fe6f2a6a02c1.1736923025.git.unicorn_wang@outlook.com
OK, I'll do as you say.
Regards,
Chen
>>> Signed-off-by: Chen Wang <unicorn_wang@...look.com>
>>> ---
>>> drivers/pci/controller/cadence/pcie-cadence-host.c | 2 +-
>>> drivers/pci/controller/cadence/pcie-cadence.c | 4 ++--
>>> drivers/pci/controller/cadence/pcie-cadence.h | 6 +++---
>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> index 8af95e9da7ce..9b9d7e722ead 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>>> @@ -452,7 +452,7 @@ static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
>>> cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(0), addr1);
>>> cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(0), desc1);
>>> - if (pcie->ops->cpu_addr_fixup)
>>> + if (pcie->ops && pcie->ops->cpu_addr_fixup)
>>> cpu_addr = pcie->ops->cpu_addr_fixup(pcie, cpu_addr);
>>> addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(12) |
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
>>> index 204e045aed8c..56c3d6cdd70e 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence.c
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence.c
>>> @@ -90,7 +90,7 @@ void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 busnr, u8 fn,
>>> cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), desc1);
>>> /* Set the CPU address */
>>> - if (pcie->ops->cpu_addr_fixup)
>>> + if (pcie->ops && pcie->ops->cpu_addr_fixup)
>>> cpu_addr = pcie->ops->cpu_addr_fixup(pcie, cpu_addr);
>>> addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) |
>>> @@ -120,7 +120,7 @@ void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie,
>>> }
>>> /* Set the CPU address */
>>> - if (pcie->ops->cpu_addr_fixup)
>>> + if (pcie->ops && pcie->ops->cpu_addr_fixup)
>>> cpu_addr = pcie->ops->cpu_addr_fixup(pcie, cpu_addr);
>>> addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(17) |
>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
>>> index f5eeff834ec1..436630d18fe0 100644
>>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>>> @@ -499,7 +499,7 @@ static inline u32 cdns_pcie_ep_fn_readl(struct cdns_pcie *pcie, u8 fn, u32 reg)
>>> static inline int cdns_pcie_start_link(struct cdns_pcie *pcie)
>>> {
>>> - if (pcie->ops->start_link)
>>> + if (pcie->ops && pcie->ops->start_link)
>>> return pcie->ops->start_link(pcie);
>>> return 0;
>>> @@ -507,13 +507,13 @@ static inline int cdns_pcie_start_link(struct cdns_pcie *pcie)
>>> static inline void cdns_pcie_stop_link(struct cdns_pcie *pcie)
>>> {
>>> - if (pcie->ops->stop_link)
>>> + if (pcie->ops && pcie->ops->stop_link)
>>> pcie->ops->stop_link(pcie);
>>> }
>>> static inline bool cdns_pcie_link_up(struct cdns_pcie *pcie)
>>> {
>>> - if (pcie->ops->link_up)
>>> + if (pcie->ops && pcie->ops->link_up)
>>> return pcie->ops->link_up(pcie);
>>> return true;
>>>
>>> base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
Powered by blists - more mailing lists