netdev - Re: [net PATCH] octeontx2-af: Fix pointer dereference before sanity check

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] [thread-next>] [day] [month] [year] [list]

Date: Wed, 7 Jun 2023 12:04:40 +0000
From: Sai Krishna Gajula <saikrishnag@...vell.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com"
	<edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        "dan.carpenter@...aro.org" <dan.carpenter@...aro.org>,
        Naveen Mamindlapalli
	<naveenm@...vell.com>
Subject: Re: [net PATCH] octeontx2-af: Fix pointer dereference before sanity
 check


> -----Original Message-----
> From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> Sent: Wednesday, June 7, 2023 5:17 PM
> To: Sai Krishna Gajula <saikrishnag@...vell.com>
> Cc: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; Sunil Kovvuri Goutham <sgoutham@...vell.com>;
> dan.carpenter@...aro.org; Naveen Mamindlapalli <naveenm@...vell.com>
> Subject: Re: [net PATCH] octeontx2-af: Fix pointer dereference before
> sanity check
> 
> On Wed, Jun 07, 2023 at 12:32:55PM +0530, Sai Krishna wrote:
> > PTP pointer is being dereferenced before NULL, error check.
> > Fixed the same to avoid NULL dereference and smatch checker warning.
> 
> please use imperative mood, you could say:
> Move validation of ptp pointer before its usage
> 
I will change in V2 patch.

> >
> > Fixes: 2ef4e45d99b1 ("octeontx2-af: Add PTP PPS Errata workaround on
> CN10K silicon")
> > Signed-off-by: Sai Krishna <saikrishnag@...vell.com>
> > Signed-off-by: Naveen Mamindlapalli <naveenm@...vell.com>
> 
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> 
> > ---
> >  drivers/net/ethernet/marvell/octeontx2/af/ptp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> b/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> > index 3411e2e47d46..6a7dfb181fa8 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/af/ptp.c
> > @@ -449,12 +449,12 @@ static void ptp_remove(struct pci_dev *pdev)
> >  	struct ptp *ptp = pci_get_drvdata(pdev);
> >  	u64 clock_cfg;
> >
> > -	if (cn10k_ptp_errata(ptp) && hrtimer_active(&ptp->hrtimer))
> > -		hrtimer_cancel(&ptp->hrtimer);
> > -
> >  	if (IS_ERR_OR_NULL(ptp))
> >  		return;
> >
> > +	if (cn10k_ptp_errata(ptp) && hrtimer_active(&ptp->hrtimer))
> > +		hrtimer_cancel(&ptp->hrtimer);
> > +
> >  	/* Disable PTP clock */
> >  	clock_cfg = readq(ptp->reg_base + PTP_CLOCK_CFG);
> >  	clock_cfg &= ~PTP_CLOCK_CFG_PTP_EN;
> 
> i wonder if ptp_remove() would be able to free the struct ptp that
> ptp_probe() allocated - then you wouldn't have to use devm_kzalloc().
> 
We intend to use devm_kzalloc() so that we do not need to call kfree in the remove function. Please let us know why you prefer to manually free the resource.

Thanks,
Sai
> > --
> > 2.25.1
> >
> >