From: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
To: Sai Krishna Gajula <saikrishnag@marvell•com>
Cc: "davem@davemloft•net" <davem@davemloft•net>,
"edumazet@google•com" <edumazet@google•com>,
"kuba@kernel•org" <kuba@kernel•org>,
"pabeni@redhat•com" <pabeni@redhat•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"linux-kernel@vger•kernel.org" <linux-kernel@vger•kernel.org>,
Sunil Kovvuri Goutham <sgoutham@marvell•com>,
"dan.carpenter@linaro•org" <dan.carpenter@linaro•org>,
Naveen Mamindlapalli <naveenm@marvell•com>
Subject: Re: [net PATCH] octeontx2-af: Fix pointer dereference before sanity check
Date: Wed, 7 Jun 2023 14:24:51 +0200 [thread overview]
Message-ID: <ZIB3E+nyJE6YIcge@boxer> (raw)
In-Reply-To: <BY3PR18MB47075B84527934A80F4B4C93A053A@BY3PR18MB4707.namprd18.prod.outlook.com>
On Wed, Jun 07, 2023 at 12:04:40PM +0000, Sai Krishna Gajula wrote:
>
> > -----Original Message-----
> > From: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
> > Sent: Wednesday, June 7, 2023 5:17 PM
> > To: Sai Krishna Gajula <saikrishnag@marvell•com>
> > Cc: davem@davemloft•net; edumazet@google•com; kuba@kernel•org;
> > pabeni@redhat•com; netdev@vger•kernel.org; linux-
> > kernel@vger•kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell•com>;
> > dan.carpenter@linaro•org; Naveen Mamindlapalli <naveenm@marvell•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@marvell•com>
> > > Signed-off-by: Naveen Mamindlapalli <naveenm@marvell•com>
> >
> > Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel•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.
I just don't think this is really necessary as this object's lifetime
scope is clearly defined, i am in the rush now but i can try to come up
with further arguments later on if needed.
>
> Thanks,
> Sai
> > > --
> > > 2.25.1
> > >
> > >
prev parent reply other threads:[~2023-06-07 12:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-07 7:02 [net PATCH] octeontx2-af: Fix pointer dereference before sanity check Sai Krishna
2023-06-07 11:46 ` Maciej Fijalkowski
2023-06-07 12:04 ` Sai Krishna Gajula
2023-06-07 12:24 ` Maciej Fijalkowski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZIB3E+nyJE6YIcge@boxer \
--to=maciej.fijalkowski@intel$(echo .)com \
--cc=dan.carpenter@linaro$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=naveenm@marvell$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=saikrishnag@marvell$(echo .)com \
--cc=sgoutham@marvell$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox