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.srikanteshw...@broadcom.com>
> Signed-off-by: Kashyap Desai <kashyap.de...@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?
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@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)
