Re: [PATCH] Staging: net: nic: Add error pointer check in otx2_flows.c
From: Dipendra Khadka
Date: Thu Sep 26 2024 - 01:31:17 EST
Hi Simon,
On Mon, 23 Sept 2024 at 21:48, Dipendra Khadka <kdipendra88@xxxxxxxxx> wrote:
>
> Hi,
>
> Thank you for the response.I had already sent the v2 patch. I will
> send v3 addressing all the issues.
>
> On Mon, 23 Sept 2024 at 21:41, Simon Horman <horms@xxxxxxxxxx> wrote:
> >
> > On Sun, Sep 22, 2024 at 06:52:35PM +0000, Dipendra Khadka wrote:
> > > Smatch reported following:
> > > '''
> > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:123 otx2_alloc_mcam_entries() error: 'rsp' dereferencing possible ERR_PTR()
> > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:201 otx2_mcam_entry_init() error: 'rsp' dereferencing possible ERR_PTR()
> > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c:236 otx2_mcam_entry_init() error: 'frsp' dereferencing possible ERR_PTR()
> > > '''
> > >
> > > Adding error pointer check after calling otx2_mbox_get_rsp.
> > >
> > > Signed-off-by: Dipendra Khadka <kdipendra88@xxxxxxxxx>
> >
> > Hi Dipendra,
> >
> > As noted by Andrew Lunn in relation to another patch [1],
> > this driver isn't in Staging so the subject is not correct.
> > And moreover, as Andrew suggested, please take a look at [2].
> >
> > [1] https://lore.kernel.org/all/13fbb6c3-661f-477a-b33b-99303cd11622@xxxxxxx/
> > [2] https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> >
>
> > > ---
> > > .../ethernet/marvell/octeontx2/nic/otx2_flows.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > > index 98c31a16c70b..4b61236c7c41 100644
> > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> > > @@ -120,6 +120,11 @@ int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
> > > rsp = (struct npc_mcam_alloc_entry_rsp *)otx2_mbox_get_rsp
> > > (&pfvf->mbox.mbox, 0, &req->hdr);
> >
> > nit: No blank line here please.
> > Similarly in the other hunks of this patch.
> >
> > >
> > > + if (IS_ERR(rsp)) {
> > > + mutex_unlock(&bfvf->mbox.lock);
> >
> > This doesn't compile as bfvf doesn't exit in this context.
> >
> > > + return PTR_ERR(rsp);
> >
> > Looking at error handling elsewhere in the same loop, perhaps this
> > is appropriate instead of returning.
> >
> > goto exit;
Is this ok to follow?
if (IS_ERR(rsp)) {
allocated = PTR_ERR(rsp);
goto exit;
}
> >
> > > + }
> > > +
> > > for (ent = 0; ent < rsp->count; ent++)
> > > flow_cfg->flow_ent[ent + allocated] = rsp->entry_list[ent];
> > >
> >
> > ...
>
> Best Regards,
> Dipendra
Best regards,
Dipendra Khadka