> -----Original Message-----
> From: Bart Van Assche <bvanassche@xxxxxxx>
> Sent: Thursday, May 18, 2023 11:12 PM
> To: Nilesh Javali <njavali@xxxxxxxxxxx>; martin.petersen@xxxxxxxxxx
> Cc: linux-scsi@xxxxxxxxxxxxxxx; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@xxxxxxxxxxx>; Bikash Hazarika <bhazarika@xxxxxxxxxxx>;
> Anil Gurumurthy <agurumurthy@xxxxxxxxxxx>; Shreyas Deodhar
> <sdeodhar@xxxxxxxxxxx>
> Subject: [EXT] Re: [PATCH 2/8] qla2xxx: klocwork - Fix potential null pointer
> dereference
>
> External Email
>
> ----------------------------------------------------------------------
> On 5/18/23 00:58, Nilesh Javali wrote:
> > From: Bikash Hazarika <bhazarika@xxxxxxxxxxx>
> >
> > Klocwork tool reported 'cur_dsd' may be dereferenced.
> > Add fix to validate pointer before dereferencing
> > the pointer.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Bikash Hazarika <bhazarika@xxxxxxxxxxx>
> > Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
> > ---
> > drivers/scsi/qla2xxx/qla_iocb.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> > index 6acfdcc48b16..a092151aef77 100644
> > --- a/drivers/scsi/qla2xxx/qla_iocb.c
> > +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> > @@ -664,9 +664,11 @@ qla24xx_build_scsi_type_6_iocbs(srb_t *sp, struct
> cmd_type_6 *cmd_pkt,
> > }
> >
> > /* Null termination */
> > - cur_dsd->address = 0;
> > - cur_dsd->length = 0;
> > - cur_dsd++;
> > + if (cur_dsd) {
> > + cur_dsd->address = 0;
> > + cur_dsd->length = 0;
> > + cur_dsd++;
> > + }
> > cmd_pkt->control_flags |=
> cpu_to_le16(CF_DATA_SEG_DESCR_ENABLE);
> > return 0;
> > }
>
> Please add BUG_ON(!cur_dsd) above the first cur_dsd dereference instead
> of making the above change. The above change hides a bug. Hiding bugs
> doesn't help anyone.
>
> Bart.
Thanks for the review.
We can prevent the crash and notify the occurrence of this
rare case by adding warn_on like,
+ WARN_ON_ONCE(!cur_dsd);
+ if (cur_dsd) {
+ cur_dsd->address = 0;
+ cur_dsd->length = 0;
+ cur_dsd++;
+ }
cmd_pkt->control_flags |= cpu_to_le16(CF_DATA_SEG_DESCR_ENABLE);
return 0;
}
Thanks,
Nilesh
