> -----Original Message----- > From: Hannes Reinecke [mailto:hare@xxxxxxxx] > Sent: Monday, February 06, 2017 4:11 PM > To: Shivasharan S; linux-scsi@xxxxxxxxxxxxxxx > Cc: martin.petersen@xxxxxxxxxx; thenzl@xxxxxxxxxx; > jejb@xxxxxxxxxxxxxxxxxx; kashyap.desai@xxxxxxxxxxxx; > sumit.saxena@xxxxxxxxxxxx > Subject: Re: [PATCH 06/39] megaraid_sas: RAID map is accessed for SYS PDs > when use_seqnum_jbod_fp is not set > > On 02/06/2017 10:59 AM, Shivasharan S wrote: > > Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@xxxxxxxxxxxx> > > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx> > > --- > > drivers/scsi/megaraid/megaraid_sas_base.c | 25 > > ++++++++++++++----------- > > 1 file changed, 14 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c > > b/drivers/scsi/megaraid/megaraid_sas_base.c > > index 6ca49ef..67a205a 100644 > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c > > @@ -1756,28 +1756,31 @@ void megasas_update_sdev_properties(struct > scsi_device *sdev) > > fusion = instance->ctrl_context; > > mr_device_priv_data = sdev->hostdata; > > > > - if (!fusion) > > + if (!fusion || !mr_device_priv_data) > > return; > > > > - if (!MEGASAS_IS_LOGICAL(sdev) && > > - instance->use_seqnum_jbod_fp) { > > - pd_index = (sdev->channel * > MEGASAS_MAX_DEV_PER_CHANNEL) + > > - sdev->id; > > - pd_sync = (void *)fusion->pd_seq_sync > > - [(instance->pd_seq_map_id - 1) & 1]; > > - mr_device_priv_data->is_tm_capable = > > - pd_sync->seq[pd_index].capability.tmCapable; > > - } else { > > + if (MEGASAS_IS_LOGICAL(sdev)) { > > device_id = ((sdev->channel % 2) * > MEGASAS_MAX_DEV_PER_CHANNEL) > > + sdev->id; > > local_map_ptr = fusion->ld_drv_map[(instance->map_id & 1)]; > > ld = MR_TargetIdToLdGet(device_id, local_map_ptr); > > + if (ld >= instance->fw_supported_vd_count) > > + return; > > raid = MR_LdRaidGet(ld, local_map_ptr); > > > > if (raid->capability.ldPiMode == > MR_PROT_INFO_TYPE_CONTROLLER) > > - blk_queue_update_dma_alignment(sdev->request_queue, 0x7); > > + blk_queue_update_dma_alignment(sdev- > >request_queue, > > + 0x7); > > + > > mr_device_priv_data->is_tm_capable = > > raid->capability.tmCapable; > > + } else if (instance->use_seqnum_jbod_fp) { > > + pd_index = (sdev->channel * > MEGASAS_MAX_DEV_PER_CHANNEL) + > > + sdev->id; > > + pd_sync = (void *)fusion->pd_seq_sync > > + [(instance->pd_seq_map_id - 1) & 1]; > > + mr_device_priv_data->is_tm_capable = > > + pd_sync->seq[pd_index].capability.tmCapable; > > } > > } > > > > > Don't you need a sanity check on pd_index, too? > Otherwise you easily overflow the ->seq array ... "pd_index" sanity check is not really required here. pd_index is function of "sdev->id", "sdev->channel" and MEGASAS_MAX_DEV_PER_CHANNEL. Here sdev->channel can either be 0 or 1 as it is under "!MEGASAS_IS_LOGICAL(sdev)", and sdev->id cannot be greater than 127(MEGASAS_MAX_DEV_PER_CHANNEL -1). So maximum possible value of pd_index is 255(256th element) and fusion->pd_seq_sync->seq array size is 256. So pd_index should not overflow fusion->pd_seq_sync->seq array. > > Cheers, > > Hannes > -- > Dr. Hannes Reinecke zSeries & Storage > hare@xxxxxxxx +49 911 74053 688 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton HRB 21284 (AG > Nürnberg)
