linux-kernel - Re: [PATCH] pch_gbe: Add NULL check for ptp_pdev in pch_gbe

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <487ba043-ba5b-4071-ad84-2cdc8ef2eaf1@linux.dev>
Date: Thu, 24 Jul 2025 09:28:54 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Chenyuan Yang <chenyuan0y@...il.com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, richardcochran@...il.com,
 mingo@...nel.org, tglx@...utronix.de, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] pch_gbe: Add NULL check for ptp_pdev in pch_gbe_probe()

On 23/07/2025 17:29, Chenyuan Yang wrote:
> On Wed, Jul 23, 2025 at 2:37 AM Vadim Fedorenko
> <vadim.fedorenko@...ux.dev> wrote:
>>
>> On 23/07/2025 04:41, Chenyuan Yang wrote:
>>> Since pci_get_domain_bus_and_slot() can return NULL for PCI_DEVFN(12, 4),
>>> add NULL check for adapter->ptp_pdev in pch_gbe_probe().
>>>
>>> This change is similar to the fix implemented in commit 9af152dcf1a0
>>> ("drm/gma500: Add NULL check for pci_gfx_root in mid_get_vbt_data()").
>>>
>>> Signed-off-by: Chenyuan Yang <chenyuan0y@...il.com>
>>> ---
>>>    drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
>>> index e5a6f59af0b6..10b8f1fea1a2 100644
>>> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
>>> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
>>> @@ -2515,6 +2515,11 @@ static int pch_gbe_probe(struct pci_dev *pdev,
>>>                pci_get_domain_bus_and_slot(pci_domain_nr(adapter->pdev->bus),
>>>                                            adapter->pdev->bus->number,
>>>                                            PCI_DEVFN(12, 4));
>>> +     if (!adapter->ptp_pdev) {
>>> +             dev_err(&pdev->dev, "PTP device not found\n");
>>> +             ret = -ENODEV;
>>> +             goto err_free_netdev;
>>> +     }
>>
>> Why is this error fatal? I believe the device still can transmit and
>> receive packets without PTP device. If this situation is really possible
>> I would suggest you to add checks to ioctl function to remove
>> timestamping support if there is no PTP device found
> 
> Thanks for the prompt reply!
> Our static analysis tool found this issue and we made the initial
> patch based on the existings checks for pci_get_domain_bus_and_slot()
> 
> I've drafted a new version based on your suggestion. It removes the
> check from the probe function and instead adds the necessary NULL
> checks directly to the timestamping and ioctl functions.
> 
> Does the implementation below look correct to you? If so, I will
> prepare and send a formal v2 patch.
> 

I would say this patch looks way too defensive. It's enough to deny
enabling HW timestamping when there is no PTP device, then all other
checks will never happen. And I would keep the error message just to
inform users that PTP feature is not available on the particular
device/system.

Do you have HW to test your patch?