Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Sai Krishna Gajula <saikrishnag@marvell.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@intel.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
Thread-Topic: [net PATCH] octeontx2-af: Fix pointer dereference before sanity
 check
Thread-Index: AQHZmTg8tSrDR18MGkOCDKfPn98TFw==
Date:   Wed, 7 Jun 2023 12:04:40 +0000
Message-ID: <BY3PR18MB47075B84527934A80F4B4C93A053A@BY3PR18MB4707.namprd18.prod.outlook.com>
References: <20230607070255.2013980-1-saikrishnag@marvell.com>
 <ZIBuIXH2M1KbCg06@boxer>
In-Reply-To: <ZIBuIXH2M1KbCg06@boxer>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?fVmbZNvtZk2b8kOxPbjQ1h5CbhuSVSiFLcsX4LQEqTTfophh9shgAU+wen57?=
 =?us-ascii?Q?DpNhK7qZNsbfBGBJgMUziTXZGSLfKCXoafkb+Oj8COKB7Hy31Rl2YxvhomGw?=
 =?us-ascii?Q?AVmACPrXG9J5BJr8GDybBICLbPG1jnYcaY/7e84Q8JyazmUFDnNE3ZG3TPBR?=
 =?us-ascii?Q?HqD29oeSghF5jXmVQcEgLMUm7OxUAsPn25gpi759XybCv5V8/UdzHcODT3y5?=
 =?us-ascii?Q?9peq6UMX7pez9EzY4cqVcRncES/7wT3dyG1EOxu02TN4dM7mWafIijRzMwMR?=
 =?us-ascii?Q?WYTsUte9qUpHUnzqHowGbHkse3i9K6Nbu+mpWH3m8Issi63Y5REUnKmJGSJk?=
 =?us-ascii?Q?BSXoe7A0f0JzdUSYUwm3hCErWxsYwsXBp5iYSk3HylNh9mGKVc4Qsga22mg+?=
 =?us-ascii?Q?UY4LuWga3jFps4lKVahnIacC+Tdj5xcFfEHAfUU7xqekPxtlOzObd/AntNGC?=
 =?us-ascii?Q?G8SG9XGIi/J8swGiYD5CH6aS5pl4etNEv2Rfum75JOQM5BzkcSyx6QMHO7ME?=
 =?us-ascii?Q?aHJ2yFpI/SqqfeKWfSL8B3wXtwr5BxVSK213pFhvHJg9oDVYupiQQZ2uyw/V?=
 =?us-ascii?Q?Pda6KRLYKEVzq2bFh3itUx5diDU9YIRruWGyrEdis2WpP/fwY7ZpLDhQ02op?=
 =?us-ascii?Q?SYHjrLBgdeZWd6MWBDJMjXFHKQ9Wa4XqnEn3Vfn9ZFSBxgnaqcEknBNnUumy?=
 =?us-ascii?Q?y/ZNQleo3zi8+72PR5purroJoWKz8nxdxSCoPT6sVkYFqKdQxTw+9Tn+2qYR?=
 =?us-ascii?Q?ldHg9QGtuGCvF2i7AjenKqR6F2lD8wSZMIkCQzABL+UKIyGoLf63qHlaJoEH?=
 =?us-ascii?Q?Mapns/nvt024a5MBaEHgmfGAqctKnvpeBaCGn5150vF/uJ31jzJTFjWBty+L?=
 =?us-ascii?Q?XDLEDU8Sx+cHI/AUOIk+6yuHib5SkAevfOkdrTNQzBUZqpANr/vnRDyh3A2X?=
 =?us-ascii?Q?BKEzkRRGZRapUI2gSwNMgz8aGgdu2VHdI05bZPe8ayVzsaxr64q2Bd2Xbsy/?=
 =?us-ascii?Q?bVt6Y7VrWVUQM5lISVMsPxYONwyxKSlCMvomhRvZV92Fuutf/DqlC0sXFLm9?=
 =?us-ascii?Q?PxZHxhK0jrxm8FBMCuA5g1l1qcPZKLtk/Bea1vYmjidMS1uLtmQlNKERymPl?=
 =?us-ascii?Q?jKnegT9xFqkv56YRDP5F/fys6K6FLIbpjieudqUc2efXofRBWdRzkrOl6f8A?=
 =?us-ascii?Q?d7BJw9tM/cWv8WFm44aZFxtxxHNoXjDkT6hjyWFn4qtzwcZQSx8F06P514kX?=
 =?us-ascii?Q?KaPpZPt7UfWZsOPnoZMWNgDD4b2E/fA0M4EoQwaaTYK2sP4d1Q0C4O37TIQM?=
 =?us-ascii?Q?eeyoIeiqaIkEbs8WXoCGspQxQZf2oRGTIuxJ9SUn1rj2KLW/q20k/yQRbOJH?=
 =?us-ascii?Q?iwcHx/bJRMBGtIyc6q3c85Z/UWN4TWEK2i+isAp/grFDRrz5jO7lvPaIk9EE?=
 =?us-ascii?Q?RPL3+V+S1BNmXsVo4ABRlNMOuYtFaWZHBpA3rqSe5TtcY6ByJzzcOjEOvENg?=
 =?us-ascii?Q?iNCSaTd0rVmd8CXlW4CEH8WQpT0coH2wD5zy0/bJbCAW0d7MKxsG2o9bb+gC?=
 =?us-ascii?Q?C8hbwY9f9Zv0rkyk7q0TrvnRwG4axCJ8we8hGGdN?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BY3PR18MB4707.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b57f98cd-1755-43ab-912f-08db674f5f5c
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jun 2023 12:04:40.2426
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: xRDwT/zdD5siLiQRMxbNhPajvDq5+nkg6zIAFXgaUA+agefJ6+aUTbGPrlRQAWBeDGY+LfEIdkkhmsuY10xhiA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR18MB5501
X-Proofpoint-ORIG-GUID: aHIcJVSteN5WpUwGFfi6OjSEwYFWi6Eh
X-Proofpoint-GUID: aHIcJVSteN5WpUwGFfi6OjSEwYFWi6Eh
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26
 definitions=2023-06-07_06,2023-06-07_01,2023-05-22_02
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----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
>=20
> 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.
>=20
> please use imperative mood, you could say:
> Move validation of ptp pointer before its usage
>=20
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>
>=20
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>=20
> > ---
> >  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 =3D 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 =3D readq(ptp->reg_base + PTP_CLOCK_CFG);
> >  	clock_cfg &=3D ~PTP_CLOCK_CFG_PTP_EN;
>=20
> 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().
>=20
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 re=
source.

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