RE: [PATCH V3 08/25] smartpqi: add support for long firmware version
From: <Don.Brace@microchip.com>
Date: 2021-01-11 22:26:33
-----Original Message----- From: Martin Wilck [mailto:mwilck@suse.com] Subject: Re: [PATCH V3 08/25] smartpqi: add support for long firmware version
quoted hunk ↗ jump to hunk
@@ -1405,7 +1405,7 @@ enum pqi_ctrl_mode { structbmic_identify_controller { u8 configured_logical_drive_count; __le32 configuration_signature; - u8 firmware_version[4]; + u8 firmware_version_short[4]; u8 reserved[145]; __le16 extended_logical_unit_count; u8 reserved1[34];@@ -1413,11 +1413,17 @@ struct bmic_identify_controller { u8 reserved2[8]; u8 vendor_id[8]; u8 product_id[16]; - u8 reserved3[68]; + u8 reserved3[62]; + __le32 extra_controller_flags; + u8 reserved4[2]; u8 controller_mode; - u8 reserved4[32]; + u8 spare_part_number[32]; + u8 firmware_version_long[32]; }; --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.cpqi_get_ctrl_product_details(struct pqi_ctrl_info *ctrl_info) if (rc) goto out; + if (get_unaligned_le32(&identify->extra_controller_flags) & + BMIC_IDENTIFY_EXTRA_FLAGS_LONG_FW_VERSION_SUPPORTED) { + memcpy(ctrl_info->firmware_version, + identify->firmware_version_long, + sizeof(identify->firmware_version_long)); + } else { + memcpy(ctrl_info->firmware_version, + identify->firmware_version_short, + sizeof(identify->firmware_version_short)); + ctrl_info->firmware_version + [sizeof(identify->firmware_version_short)] = '\0'; + snprintf(ctrl_info->firmware_version + + strlen(ctrl_info->firmware_version), + sizeof(ctrl_info->firmware_version),
This looks wrong. I suppose a real overflow can't happen, but shouldn't it rather be written like this? snprintf(ctrl_info->firmware_version + sizeof(identify->firmware_version_short), sizeof(ctrl_info->firmware_version) - sizeof(identify->firmware_version_short), "-u", ...)
+ "-%u", + get_unaligned_le16(&identify-quoted
firmware_build_number));
Don: Agreed. Updated. Thanks for your revew.