From: Shivasharan Srikanteshwara <shivasharan.srikanteshwara@broadcom.com>
To: Hannes Reinecke <hare@suse.com>, linux-scsi@vger.kernel.org
Cc: martin.petersen@oracle.com, thenzl@redhat.com,
jejb@linux.vnet.ibm.com,
Kashyap Desai <kashyap.desai@broadcom.com>,
Sumit Saxena <sumit.saxena@broadcom.com>
Subject: RE: [PATCH v3 19/39] megaraid_sas: MR_TargetIdToLdGet u8 to u16 and avoid invalid raid-map access
Date: Fri, 10 Feb 2017 17:34:03 +0530 [thread overview]
Message-ID: <389bd90e6f4a50671e69c7a76fbe9598@mail.gmail.com> (raw)
In-Reply-To: <b64c949b-dbd3-2bea-edee-b2c50f0126bd@suse.com>
> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@suse.com]
> Sent: Friday, February 10, 2017 5:05 PM
> To: Shivasharan S; linux-scsi@vger.kernel.org
> Cc: martin.petersen@oracle.com; thenzl@redhat.com;
> jejb@linux.vnet.ibm.com; kashyap.desai@broadcom.com;
> sumit.saxena@broadcom.com
> Subject: Re: [PATCH v3 19/39] megaraid_sas: MR_TargetIdToLdGet u8 to u16
> and avoid invalid raid-map access
>
> On 02/10/2017 09:59 AM, Shivasharan S wrote:
> > Change MR_TargetIdToLdGet return type from u8 to u16.
> >
> > ld id range check is added at two places in this patch -
> > @megasas_build_ldio_fusion and @megasas_build_ld_nonrw_fusion.
> > Previous driver code used different data type for lds TargetId
returned from
> MR_TargetIdToLdGet.
> > Prior to this change, above two functions was safeguarded due to
> > function always return u8 and maximum value of ld id returned was 255.
> >
> > In below check, fw_supported_vd_count as of today is 64 or 256 and
> > valid range to support is either 0-63 or 0-255. Ideally want to
> > filter accessing raid map for ld ids which are not valid. With the u16
> > change, invalid ld id value is 0xFFFF and we will see kernel panic due
to random
> memory access in MR_LdRaidGet.
> > The changes will ensure we do not call MR_LdRaidGet if ld id is beyond
size of
> ldSpanMap array.
> >
> > if (ld < instance->fw_supported_vd_count)
> >
> > From firmware perspective,ld id 0xFF is invalid and even though
> > current driver code forward such command, firmware fails with target
not
> available.
> >
> > ld target id issue occurs mainly whenever driver loops to populate
raid map
> (ea. MR_ValidateMapInfo).
> > These are the only two places where we may see out of range target ids
> > and wants to protect raid map access based on range provided by
Firmware
> API.
> >
> > Signed-off-by: Shivasharan S <shivasharan.srikanteshwara@broadcom.com>
> > Signed-off-by: Kashyap Desai <kashyap.desai@broadcom.com>
> > ---
> >
> > fix in v2 - updated description content.
> >
> > drivers/scsi/megaraid/megaraid_sas.h | 2 +-
> > drivers/scsi/megaraid/megaraid_sas_fp.c | 5 +++--
> > drivers/scsi/megaraid/megaraid_sas_fusion.c | 25
> > ++++++++++++++-----------
> > 3 files changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> > b/drivers/scsi/megaraid/megaraid_sas.h
> > index 0a20fff..efc01a3 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas.h
> > +++ b/drivers/scsi/megaraid/megaraid_sas.h
> > @@ -2448,7 +2448,7 @@ MR_BuildRaidContext(struct megasas_instance
> *instance,
> > struct IO_REQUEST_INFO *io_info,
> > struct RAID_CONTEXT *pRAID_Context,
> > struct MR_DRV_RAID_MAP_ALL *map, u8 **raidLUN);
> > -u8 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL *map);
> > +u16 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL
> *map);
> > struct MR_LD_RAID *MR_LdRaidGet(u32 ld, struct MR_DRV_RAID_MAP_ALL
> > *map);
> > u16 MR_ArPdGet(u32 ar, u32 arm, struct MR_DRV_RAID_MAP_ALL *map);
> > u16 MR_LdSpanArrayGet(u32 ld, u32 span, struct MR_DRV_RAID_MAP_ALL
> > *map); diff --git a/drivers/scsi/megaraid/megaraid_sas_fp.c
> > b/drivers/scsi/megaraid/megaraid_sas_fp.c
> > index a0b0e68..9d5d485 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_fp.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_fp.c
> > @@ -165,7 +165,7 @@ u16 MR_GetLDTgtId(u32 ld, struct
> MR_DRV_RAID_MAP_ALL *map)
> > return le16_to_cpu(map->raidMap.ldSpanMap[ld].ldRaid.targetId);
> > }
> >
> > -u8 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL *map)
> > +u16 MR_TargetIdToLdGet(u32 ldTgtId, struct MR_DRV_RAID_MAP_ALL
> *map)
> > {
> > return map->raidMap.ldTgtIdToLd[ldTgtId];
> > }
> > @@ -1151,7 +1151,7 @@ MR_BuildRaidContext(struct megasas_instance
> > *instance, {
> > struct fusion_context *fusion;
> > struct MR_LD_RAID *raid;
> > - u32 ld, stripSize, stripe_mask;
> > + u32 stripSize, stripe_mask;
> > u64 endLba, endStrip, endRow, start_row, start_strip;
> > u64 regStart;
> > u32 regSize;
> > @@ -1163,6 +1163,7 @@ MR_BuildRaidContext(struct megasas_instance
> *instance,
> > u8 retval = 0;
> > u8 startlba_span = SPAN_INVALID;
> > u64 *pdBlock = &io_info->pdBlock;
> > + u16 ld;
> >
> > ldStartBlock = io_info->ldStartBlock;
> > numBlocks = io_info->numBlocks;
> > diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > index 9019b82..4aaf307 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
> > @@ -1121,7 +1121,8 @@ megasas_sync_map_info(struct megasas_instance
> *instance)
> > int i;
> > struct megasas_cmd *cmd;
> > struct megasas_dcmd_frame *dcmd;
> > - u32 size_sync_info, num_lds;
> > + u16 num_lds;
> > + u32 size_sync_info;
> > struct fusion_context *fusion;
> > struct MR_LD_TARGET_SYNC *ci = NULL;
> > struct MR_DRV_RAID_MAP_ALL *map;
> > @@ -1870,7 +1871,7 @@ megasas_set_pd_lba(struct
> MPI2_RAID_SCSI_IO_REQUEST *io_request, u8 cdb_len,
> > struct MR_DRV_RAID_MAP_ALL *local_map_ptr, u32 ref_tag)
> {
> > struct MR_LD_RAID *raid;
> > - u32 ld;
> > + u16 ld;
> > u64 start_blk = io_info->pdBlock;
> > u8 *cdb = io_request->CDB.CDB32;
> > u32 num_blocks = io_info->numBlocks; @@ -2303,10 +2304,11 @@
> > megasas_build_ldio_fusion(struct megasas_instance *instance,
> >
> > local_map_ptr = fusion->ld_drv_map[(instance->map_id & 1)];
> > ld = MR_TargetIdToLdGet(device_id, local_map_ptr);
> > - raid = MR_LdRaidGet(ld, local_map_ptr);
> >
> > - if ((MR_TargetIdToLdGet(device_id, local_map_ptr) >=
> > - instance->fw_supported_vd_count) ||
(!fusion->fast_path_io)) {
> > + if (ld < instance->fw_supported_vd_count)
> > + raid = MR_LdRaidGet(ld, local_map_ptr);
> > +
> > + if (!raid || (!fusion->fast_path_io)) {
> > io_request->RaidContext.raid_context.reg_lock_flags = 0;
> > fp_possible = false;
> > } else {
> Is 'raid' correctly set to zero if the above condition is _not_ taken?
Hi Hannes,
'raid' is being initialized to NULL in megasas_build_ldio_fusion.
The initialization code is added in patch 2 of this series which does some
refactoring of code in this function.
-Shivasharan
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke zSeries & Storage
> hare@suse.com +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)
next prev parent reply other threads:[~2017-02-10 12:05 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-10 8:59 [PATCH v3 00/39] megaraid_sas: Updates for scsi-next Shivasharan S
2017-02-10 8:59 ` [PATCH v3 01/39] Revert "scsi: megaraid_sas: Enable or Disable Fast path based on the PCI Threshold Bandwidth" Shivasharan S
2017-02-10 8:59 ` [PATCH v3 02/39] megaraid_sas: cpu select rework Shivasharan S
2017-02-10 8:59 ` [PATCH v3 03/39] megaraid_sas: raid 1 fast path code optimize Shivasharan S
2017-02-10 8:59 ` [PATCH v3 04/39] megaraid_sas: 32 bit descriptor fire cmd optimization Shivasharan S
2017-02-10 8:59 ` [PATCH v3 05/39] megaraid_sas: Refactor MEGASAS_IS_LOGICAL macro using sdev Shivasharan S
2017-02-10 8:59 ` [PATCH v3 06/39] megaraid_sas: RAID map is accessed for SYS PDs when use_seqnum_jbod_fp is not set Shivasharan S
2017-02-10 8:59 ` [PATCH v3 07/39] megaraid_sas: Use DID_REQUEUE Shivasharan S
2017-02-10 8:59 ` [PATCH v3 08/39] megaraid_sas: megasas_get_request_descriptor always return valid desc Shivasharan S
2017-02-10 8:59 ` [PATCH v3 09/39] megaraid_sas: change issue_dcmd to return void from int Shivasharan S
2017-02-10 8:59 ` [PATCH v3 10/39] megaraid_sas: NVME Interface detection and prop settings Shivasharan S
2017-02-10 8:59 ` [PATCH v3 11/39] megaraid_sas: NVME interface target prop added Shivasharan S
2017-02-10 8:59 ` [PATCH v3 12/39] megaraid_sas: NVME fast path io support Shivasharan S
2017-02-10 8:59 ` [PATCH v3 13/39] megaraid_sas: raid 1 write performance for large io Shivasharan S
2017-02-10 8:59 ` [PATCH v3 14/39] megaraid_sas: set residual bytes count during IO completion Shivasharan S
2017-02-10 8:59 ` [PATCH v3 15/39] megaraid_sas: enhance debug logs in OCR context Shivasharan S
2017-02-10 8:59 ` [PATCH v3 16/39] megaraid_sas: add print in device removal path Shivasharan S
2017-02-10 8:59 ` [PATCH v3 17/39] megaraid_sas: reduce size of fusion_context and use vmalloc if kmalloc fails Shivasharan S
2017-02-10 8:59 ` [PATCH v3 18/39] megaraid_sas: In validate raid map, raid capability is not converted to cpu format for all lds Shivasharan S
2017-02-10 8:59 ` [PATCH v3 19/39] megaraid_sas: MR_TargetIdToLdGet u8 to u16 and avoid invalid raid-map access Shivasharan S
2017-02-10 11:34 ` Hannes Reinecke
2017-02-10 12:04 ` Shivasharan Srikanteshwara [this message]
2017-02-10 13:46 ` Tomas Henzl
2017-02-10 8:59 ` [PATCH v3 20/39] megaraid_sas: Big endian RDPQ mode fix Shivasharan S
2017-02-10 8:59 ` [PATCH v3 21/39] megaraid_sas: big endian support changes Shivasharan S
2017-02-10 8:59 ` [PATCH v3 22/39] megaraid_sas: avoid unaligned access in ioctl path Shivasharan S
2017-02-10 8:59 ` [PATCH v3 23/39] megaraid_sas: latest controller OCR capability from FW before sending shutdown DCMD Shivasharan S
2017-02-10 8:59 ` [PATCH v3 24/39] megaraid_sas: set pd_after_lb from MR_BuildRaidContext and initialize pDevHandle to MR_DEVHANDLE_INVALID Shivasharan S
2017-02-10 8:59 ` [PATCH v3 25/39] megaraid_sas: Change max_cmd from u32 to u16 in all functions Shivasharan S
2017-02-10 8:59 ` [PATCH v3 26/39] megaraid_sas: update can_queue only if the new value is less Shivasharan S
2017-02-10 8:59 ` [PATCH v3 27/39] megaraid_sas: max_fw_cmds are decremented twice, remove duplicate Shivasharan S
2017-02-10 8:59 ` [PATCH v3 28/39] megaraid_sas: megasas_return_cmd does not memset IO frame to zero Shivasharan S
2017-02-10 8:59 ` [PATCH v3 29/39] megaraid_sas: Remove unused pd_index from megasas_build_ld_nonrw_fusion Shivasharan S
2017-02-10 8:59 ` [PATCH v3 30/39] megaraid_sas: Do not set fp_possible if TM capable for non-RW syspdIO, change fp_possible to bool Shivasharan S
2017-02-10 8:59 ` [PATCH v3 31/39] megaraid_sas: During OCR, if get_ctrl_info fails do not continue with OCR Shivasharan S
2017-02-10 8:59 ` [PATCH v3 32/39] megaraid_sas: Change build_mpt_mfi_pass_thru to return void Shivasharan S
2017-02-10 8:59 ` [PATCH v3 33/39] megaraid_sas: Bail out the driver load if ld_list_query fails Shivasharan S
2017-02-10 8:59 ` [PATCH v3 34/39] megaraid_sas: Use synchronize_irq to wait for IRQs to complete Shivasharan S
2017-02-10 8:59 ` [PATCH v3 35/39] megaraid_sas: Increase internal command pool Shivasharan S
2017-02-10 8:59 ` [PATCH v3 36/39] megaraid_sas: Cleanup VD_EXT_DEBUG and SPAN_DEBUG related debug prints Shivasharan S
2017-02-10 8:59 ` [PATCH v3 37/39] megaraid_sas: Indentation and smatch warning fixes Shivasharan S
2017-02-10 8:59 ` [PATCH v3 38/39] megaraid_sas: Change RAID_1_10_RMW_CMDS to RAID_1_PEER_CMDS and set value to 2 Shivasharan S
2017-02-10 8:59 ` [PATCH v3 39/39] megaraid_sas: driver version upgrade Shivasharan S
2017-02-13 12:28 ` [PATCH v3 00/39] megaraid_sas: Updates for scsi-next Martin K. Petersen
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=389bd90e6f4a50671e69c7a76fbe9598@mail.gmail.com \
--to=shivasharan.srikanteshwara@broadcom.com \
--cc=hare@suse.com \
--cc=jejb@linux.vnet.ibm.com \
--cc=kashyap.desai@broadcom.com \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=sumit.saxena@broadcom.com \
--cc=thenzl@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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