[PATCH 0/2] scsi: FDMI Fixes

[PATCH 0/2] scsi: FDMI Fixes

[Javed Hasan]

This series has two fixes for FDMI.
Attributes length corrected for RHBA.
Fixed the wrong condition check in fc_ct_ms_fill_attr().

Kindly apply this series to scsi-queue at your earliest convenience.

Javed Hasan (2):
  scsi: fc: Corrected RHBA attributes length
  libfc: Corrected the condition check and invalid argument passed

 drivers/scsi/libfc/fc_encode.h | 8 +++++---
 include/scsi/fc/fc_ms.h        | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.26.2

[PATCH 1/2] scsi: fc: Corrected RHBA attributes length

[Javed Hasan]

 -As per document of FC-GS-5, attribute lengths of node_name
  and manufacturer should in range of "4 to 64 Bytes" only.

Signed-off-by: Javed Hasan <jhasan@marvell.com>

---
 include/scsi/fc/fc_ms.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h
index 9e273fed0a85..800d53dc9470 100644
--- a/include/scsi/fc/fc_ms.h
+++ b/include/scsi/fc/fc_ms.h
@@ -63,8 +63,8 @@ enum fc_fdmi_hba_attr_type {
  * HBA Attribute Length
  */
 #define FC_FDMI_HBA_ATTR_NODENAME_LEN		8
-#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN	80
-#define FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN	80
+#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN	64
+#define FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN	64
 #define FC_FDMI_HBA_ATTR_MODEL_LEN		256
 #define FC_FDMI_HBA_ATTR_MODELDESCR_LEN		256
 #define FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN	256
-- 
2.26.2

Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length

[Himanshu Madhani]

On 6/1/21 12:21 PM, Javed Hasan wrote:
>   -As per document of FC-GS-5, attribute lengths of node_name
>    and manufacturer should in range of "4 to 64 Bytes" only.
> 
> Signed-off-by: Javed Hasan <jhasan@marvell.com>
> 
> ---
>   include/scsi/fc/fc_ms.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h
> index 9e273fed0a85..800d53dc9470 100644
> --- a/include/scsi/fc/fc_ms.h
> +++ b/include/scsi/fc/fc_ms.h
> @@ -63,8 +63,8 @@ enum fc_fdmi_hba_attr_type {
>    * HBA Attribute Length
>    */
>   #define FC_FDMI_HBA_ATTR_NODENAME_LEN		8
> -#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN	80
> -#define FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN	80
> +#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN	64
> +#define FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN	64
>   #define FC_FDMI_HBA_ATTR_MODEL_LEN		256
>   #define FC_FDMI_HBA_ATTR_MODELDESCR_LEN		256
>   #define FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN	256
> 

Looks good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani                                Oracle Linux Engineering

Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length

[Himanshu Madhani]

On 6/1/21 12:52 PM, Himanshu Madhani wrote:
> 
> 
> On 6/1/21 12:21 PM, Javed Hasan wrote:
>>   -As per document of FC-GS-5, attribute lengths of node_name
>>    and manufacturer should in range of "4 to 64 Bytes" only.
>>
>> Signed-off-by: Javed Hasan <jhasan@marvell.com>
>>
>> ---
>>   include/scsi/fc/fc_ms.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h
>> index 9e273fed0a85..800d53dc9470 100644
>> --- a/include/scsi/fc/fc_ms.h
>> +++ b/include/scsi/fc/fc_ms.h
>> @@ -63,8 +63,8 @@ enum fc_fdmi_hba_attr_type {
>>    * HBA Attribute Length
>>    */
>>   #define FC_FDMI_HBA_ATTR_NODENAME_LEN        8
>> -#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN    80
>> -#define FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN    80
>> +#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN    64
>> +#define FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN    64
>>   #define FC_FDMI_HBA_ATTR_MODEL_LEN        256
>>   #define FC_FDMI_HBA_ATTR_MODELDESCR_LEN        256
>>   #define FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN    256
>>
> 
> Looks good.
> 
> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
> 

I just noticed that this patch is basically reverting commit 
e721eb0616f62e766882b80fd3433b80635abd5f ("scsi: scsi_transport_fc: 
Match HBA Attribute Length with HBAAPI V2.0 definitions").

Have you verified that the compiler warnings do not resurface with your 
patch? if you see that compiler warning, please fix appropriately and 
resubmit this patch.

-- 
Himanshu Madhani                                Oracle Linux Engineering

RE: [EXT] Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length

[Javed Hasan]

Hello Himanshu,

There are no warnings resurfaced after this fix because second patch of this series taking care of those compiler warnings.
Verified using "make W=1" Command.

Regards
Javed

-----Original Message-----
From: Himanshu Madhani <himanshu.madhani@oracle.com> 
Sent: Tuesday, June 1, 2021 11:38 PM
To: Javed Hasan <jhasan@marvell.com>; martin.petersen@oracle.com
Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-Upstream@marvell.com>
Subject: [EXT] Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length

External Email

----------------------------------------------------------------------


On 6/1/21 12:52 PM, Himanshu Madhani wrote:
> 
> 
> On 6/1/21 12:21 PM, Javed Hasan wrote:
>>   -As per document of FC-GS-5, attribute lengths of node_name
>>    and manufacturer should in range of "4 to 64 Bytes" only.
>>
>> Signed-off-by: Javed Hasan <jhasan@marvell.com>
>>
>> ---
>>   include/scsi/fc/fc_ms.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h index 
>> 9e273fed0a85..800d53dc9470 100644
>> --- a/include/scsi/fc/fc_ms.h
>> +++ b/include/scsi/fc/fc_ms.h
>> @@ -63,8 +63,8 @@ enum fc_fdmi_hba_attr_type {
>>    * HBA Attribute Length
>>    */
>>   #define FC_FDMI_HBA_ATTR_NODENAME_LEN        8 -#define 
>> FC_FDMI_HBA_ATTR_MANUFACTURER_LEN    80 -#define 
>> FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN    80
>> +#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN    64 #define 
>> +FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN    64
>>   #define FC_FDMI_HBA_ATTR_MODEL_LEN        256
>>   #define FC_FDMI_HBA_ATTR_MODELDESCR_LEN        256
>>   #define FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN    256
>>
> 
> Looks good.
> 
> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
> 

I just noticed that this patch is basically reverting commit e721eb0616f62e766882b80fd3433b80635abd5f ("scsi: scsi_transport_fc: 
Match HBA Attribute Length with HBAAPI V2.0 definitions").

Have you verified that the compiler warnings do not resurface with your patch? if you see that compiler warning, please fix appropriately and resubmit this patch.

-- 
Himanshu Madhani                                Oracle Linux Engineering

Re: [EXT] [PATCH 1/2] scsi: fc: Corrected RHBA attributes length

[Himanshu Madhani]

> On Jun 2, 2021, at 1:01 AM, Javed Hasan <jhasan@marvell.com> wrote:
> 
> Hello Himanshu,
> 
> There are no warnings resurfaced after this fix because second patch of this series taking care of those compiler warnings.
> Verified using "make W=1" Command.
> 

In that case, Please resubmit with Fixes tag and Cc to stable. You can keep my R-b. 

Thanks.

> Regards
> Javed
> 
> -----Original Message-----
> From: Himanshu Madhani <himanshu.madhani@oracle.com> 
> Sent: Tuesday, June 1, 2021 11:38 PM
> To: Javed Hasan <jhasan@marvell.com>; martin.petersen@oracle.com
> Cc: linux-scsi@vger.kernel.org; GR-QLogic-Storage-Upstream <GR-QLogic-Storage-Upstream@marvell.com>
> Subject: [EXT] Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> On 6/1/21 12:52 PM, Himanshu Madhani wrote:
>> 
>> 
>> On 6/1/21 12:21 PM, Javed Hasan wrote:
>>>   -As per document of FC-GS-5, attribute lengths of node_name
>>>    and manufacturer should in range of "4 to 64 Bytes" only.
>>> 
>>> Signed-off-by: Javed Hasan <jhasan@marvell.com>
>>> 
>>> ---
>>>   include/scsi/fc/fc_ms.h | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/include/scsi/fc/fc_ms.h b/include/scsi/fc/fc_ms.h index 
>>> 9e273fed0a85..800d53dc9470 100644
>>> --- a/include/scsi/fc/fc_ms.h
>>> +++ b/include/scsi/fc/fc_ms.h
>>> @@ -63,8 +63,8 @@ enum fc_fdmi_hba_attr_type {
>>>    * HBA Attribute Length
>>>    */
>>>   #define FC_FDMI_HBA_ATTR_NODENAME_LEN        8 -#define 
>>> FC_FDMI_HBA_ATTR_MANUFACTURER_LEN    80 -#define 
>>> FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN    80
>>> +#define FC_FDMI_HBA_ATTR_MANUFACTURER_LEN    64 #define 
>>> +FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN    64
>>>   #define FC_FDMI_HBA_ATTR_MODEL_LEN        256
>>>   #define FC_FDMI_HBA_ATTR_MODELDESCR_LEN        256
>>>   #define FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN    256
>>> 
>> 
>> Looks good.
>> 
>> Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
>> 
> 
> I just noticed that this patch is basically reverting commit e721eb0616f62e766882b80fd3433b80635abd5f ("scsi: scsi_transport_fc: 
> Match HBA Attribute Length with HBAAPI V2.0 definitions").
> 
> Have you verified that the compiler warnings do not resurface with your patch? if you see that compiler warning, please fix appropriately and resubmit this patch.
> 
> -- 
> Himanshu Madhani                                Oracle Linux Engineering

--
Himanshu Madhani	 Oracle Linux Engineering

[PATCH 2/2] libfc: Corrected the condition check and invalid argument passed

[Javed Hasan]

 -In correct condition check was leading to data corruption
  and so the invalid argument.

Signed-off-by: Javed Hasan <jhasan@marvell.com>

---
 drivers/scsi/libfc/fc_encode.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libfc/fc_encode.h b/drivers/scsi/libfc/fc_encode.h
index 602c97a651bc..9ea4ceadb559 100644
--- a/drivers/scsi/libfc/fc_encode.h
+++ b/drivers/scsi/libfc/fc_encode.h
@@ -166,9 +166,11 @@ static inline int fc_ct_ns_fill(struct fc_lport *lport,
 static inline void fc_ct_ms_fill_attr(struct fc_fdmi_attr_entry *entry,
 				    const char *in, size_t len)
 {
-	int copied = strscpy(entry->value, in, len);
-	if (copied > 0)
-		memset(entry->value, copied, len - copied);
+	int copied;
+
+	copied = strscpy((char *)&entry->value, in, len);
+	if (copied > 0 && (copied + 1) < len)
+		memset((entry->value + copied + 1), 0, len - copied - 1);
 }
 
 /**
-- 
2.26.2

Re: [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed

[Himanshu Madhani]

On 6/1/21 12:21 PM, Javed Hasan wrote:
>   -In correct condition check was leading to data corruption
>    and so the invalid argument.
> 
> Signed-off-by: Javed Hasan <jhasan@marvell.com>
> 
> ---
>   drivers/scsi/libfc/fc_encode.h | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/libfc/fc_encode.h b/drivers/scsi/libfc/fc_encode.h
> index 602c97a651bc..9ea4ceadb559 100644
> --- a/drivers/scsi/libfc/fc_encode.h
> +++ b/drivers/scsi/libfc/fc_encode.h
> @@ -166,9 +166,11 @@ static inline int fc_ct_ns_fill(struct fc_lport *lport,
>   static inline void fc_ct_ms_fill_attr(struct fc_fdmi_attr_entry *entry,
>   				    const char *in, size_t len)
>   {
> -	int copied = strscpy(entry->value, in, len);
> -	if (copied > 0)
> -		memset(entry->value, copied, len - copied);
> +	int copied;
> +
> +	copied = strscpy((char *)&entry->value, in, len);
> +	if (copied > 0 && (copied + 1) < len)
> +		memset((entry->value + copied + 1), 0, len - copied - 1);
>   }
>   
>   /**
> 

This should have Oracle Linux Engineering

Fixes: 8fd9efca86d08 ("scsi: libfc: Work around -Warray-bounds warning") 
  Cc: Stable@vger.kernel.org.

otherwise looks good.

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>

-- 
Himanshu Madhani                                Oracle Linux Engineering

[PATCH 0/2] scsi: FDMI Fixes

[Javed Hasan]

This series has two fixes for FDMI.
Attributes length corrected for RHBA.
Fixed the wrong condition check in fc_ct_ms_fill_attr().

Kindly apply this series to scsi-queue at your earliest convenience.

Javed Hasan (2):
  scsi: fc: Corrected RHBA attributes length
  libfc: Corrected the condition check and invalid argument passed

 drivers/scsi/libfc/fc_encode.h | 8 +++++---
 include/scsi/fc/fc_ms.h        | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

-- 
2.26.2

[PATCH 2/2] libfc: Corrected the condition check and invalid argument passed

[Javed Hasan]

 -In correct condition check was leading to data corruption
  and so the invalid argument.

Fixes: 8fd9efca86d0 ("scsi: libfc: Work around -Warray-bounds warning")
Signed-off-by: Javed Hasan <jhasan@marvell.com>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
---
 drivers/scsi/libfc/fc_encode.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libfc/fc_encode.h b/drivers/scsi/libfc/fc_encode.h
index 602c97a651bc..9ea4ceadb559 100644
--- a/drivers/scsi/libfc/fc_encode.h
+++ b/drivers/scsi/libfc/fc_encode.h
@@ -166,9 +166,11 @@ static inline int fc_ct_ns_fill(struct fc_lport *lport,
 static inline void fc_ct_ms_fill_attr(struct fc_fdmi_attr_entry *entry,
 				    const char *in, size_t len)
 {
-	int copied = strscpy(entry->value, in, len);
-	if (copied > 0)
-		memset(entry->value, copied, len - copied);
+	int copied;
+
+	copied = strscpy((char *)&entry->value, in, len);
+	if (copied > 0 && (copied + 1) < len)
+		memset((entry->value + copied + 1), 0, len - copied - 1);
 }
 
 /**
-- 
2.26.2