All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] scsi: FDMI Fixes
@ 2021-06-01 17:21 Javed Hasan
  2021-06-01 17:21 ` [PATCH 1/2] scsi: fc: Corrected RHBA attributes length Javed Hasan
  2021-06-01 17:21 ` [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed Javed Hasan
  0 siblings, 2 replies; 9+ messages in thread
From: Javed Hasan @ 2021-06-01 17:21 UTC (permalink / raw)
  To: martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream, jhasan

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
  2021-06-01 17:21 [PATCH 0/2] scsi: FDMI Fixes Javed Hasan
@ 2021-06-01 17:21 ` Javed Hasan
  2021-06-01 17:52   ` Himanshu Madhani
  2021-06-01 17:21 ` [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed Javed Hasan
  1 sibling, 1 reply; 9+ messages in thread
From: Javed Hasan @ 2021-06-01 17:21 UTC (permalink / raw)
  To: martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream, jhasan

 -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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed
  2021-06-01 17:21 [PATCH 0/2] scsi: FDMI Fixes Javed Hasan
  2021-06-01 17:21 ` [PATCH 1/2] scsi: fc: Corrected RHBA attributes length Javed Hasan
@ 2021-06-01 17:21 ` Javed Hasan
  2021-06-01 18:00   ` Himanshu Madhani
  1 sibling, 1 reply; 9+ messages in thread
From: Javed Hasan @ 2021-06-01 17:21 UTC (permalink / raw)
  To: martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream, jhasan

 -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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
  2021-06-01 17:21 ` [PATCH 1/2] scsi: fc: Corrected RHBA attributes length Javed Hasan
@ 2021-06-01 17:52   ` Himanshu Madhani
  2021-06-01 18:07     ` Himanshu Madhani
  0 siblings, 1 reply; 9+ messages in thread
From: Himanshu Madhani @ 2021-06-01 17:52 UTC (permalink / raw)
  To: Javed Hasan, martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed
  2021-06-01 17:21 ` [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed Javed Hasan
@ 2021-06-01 18:00   ` Himanshu Madhani
  0 siblings, 0 replies; 9+ messages in thread
From: Himanshu Madhani @ 2021-06-01 18:00 UTC (permalink / raw)
  To: Javed Hasan, martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
  2021-06-01 17:52   ` Himanshu Madhani
@ 2021-06-01 18:07     ` Himanshu Madhani
  2021-06-02  6:01       ` [EXT] " Javed Hasan
  0 siblings, 1 reply; 9+ messages in thread
From: Himanshu Madhani @ 2021-06-01 18:07 UTC (permalink / raw)
  To: Javed Hasan, martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [EXT] Re: [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
  2021-06-01 18:07     ` Himanshu Madhani
@ 2021-06-02  6:01       ` Javed Hasan
  2021-06-02 13:35         ` [EXT] " Himanshu Madhani
  0 siblings, 1 reply; 9+ messages in thread
From: Javed Hasan @ 2021-06-02  6:01 UTC (permalink / raw)
  To: Himanshu Madhani, martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [EXT] [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
  2021-06-02  6:01       ` [EXT] " Javed Hasan
@ 2021-06-02 13:35         ` Himanshu Madhani
  0 siblings, 0 replies; 9+ messages in thread
From: Himanshu Madhani @ 2021-06-02 13:35 UTC (permalink / raw)
  To: Javed Hasan; +Cc: Martin Petersen, linux-scsi, GR-QLogic-Storage-Upstream



> 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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] scsi: fc: Corrected RHBA attributes length
  2021-06-03  9:56 [PATCH 0/2] scsi: FDMI Fixes Javed Hasan
@ 2021-06-03  9:56 ` Javed Hasan
  0 siblings, 0 replies; 9+ messages in thread
From: Javed Hasan @ 2021-06-03  9:56 UTC (permalink / raw)
  To: martin.petersen; +Cc: linux-scsi, GR-QLogic-Storage-Upstream, jhasan

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

Fixes: e721eb0616f6 ("scsi: scsi_transport_fc: Match HBA Attribute Length with HBAAPI V2.0 definitions")
Signed-off-by: Javed Hasan <jhasan@marvell.com>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-03  9:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-01 17:21 [PATCH 0/2] scsi: FDMI Fixes Javed Hasan
2021-06-01 17:21 ` [PATCH 1/2] scsi: fc: Corrected RHBA attributes length Javed Hasan
2021-06-01 17:52   ` Himanshu Madhani
2021-06-01 18:07     ` Himanshu Madhani
2021-06-02  6:01       ` [EXT] " Javed Hasan
2021-06-02 13:35         ` [EXT] " Himanshu Madhani
2021-06-01 17:21 ` [PATCH 2/2] libfc: Corrected the condition check and invalid argument passed Javed Hasan
2021-06-01 18:00   ` Himanshu Madhani
2021-06-03  9:56 [PATCH 0/2] scsi: FDMI Fixes Javed Hasan
2021-06-03  9:56 ` [PATCH 1/2] scsi: fc: Corrected RHBA attributes length Javed Hasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.