Thread (85 messages) 85 messages, 6 authors, 2021-03-30

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 {  struct 
bmic_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.c
pqi_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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help