[PATCH 0/2] Bug fix in DevicePathLib regarding converting from text

[PATCH 0/2] Bug fix in DevicePathLib regarding converting from text

[Ruiyu Ni]

Ruiyu Ni (2):
  MdePkg/DevicePathFromText: Fix bug when converting iSCSI node
  MdePkg/DevicePathFromText: Fix byte orders when handling 8-byte array

 .../Library/UefiDevicePathLib/DevicePathFromText.c | 36 +++++++++++++---------
 1 file changed, 22 insertions(+), 14 deletions(-)

-- 
2.16.1.windows.1

[PATCH 1/2] MdePkg/DevicePathFromText: Fix bug when converting iSCSI node

[Ruiyu Ni]

If protocol string is not specified, default TCP(0) should be used.
Today's implementation wrongly sets to 1 for this case.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
---
 MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
index 0459f0ab25..c66b77ba6c 100644
--- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
@@ -1,7 +1,7 @@
 /** @file
   DevicePathFromText protocol as defined in the UEFI 2.0 specification.
 
-Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -2581,7 +2581,7 @@ DevPathFromTextiSCSI (
 
   ISCSIDevPath->LoginOption      = (UINT16) Options;
 
-  if (StrCmp (ProtocolStr, L"TCP") == 0) {
+  if (IS_NULL (*ProtocolStr) || (StrCmp (ProtocolStr, L"TCP") == 0)) {
     ISCSIDevPath->NetworkProtocol = 0;
   } else {
     //
-- 
2.16.1.windows.1

[PATCH 2/2] MdePkg/DevicePathFromText: Fix byte orders when handling 8-byte array

[Ruiyu Ni]

Per UEFI spec, FibreEx.WWN, FibreEx.Lun, SasEx.Address, SasEx.Lun
and iSCSI.Lun are all 8-byte array with byte #0 in the left.
It means "0102030405060708" should be converted to:
    UINT8[8] = {01, 02, 03, 04, 05, 06, 07, 08}
or  UINT64 = {0807060504030201}

Today's implementation wrongly uses the reversed order.
The patch fixes this issue by using StrHexToBytes().

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jie Lin <jie.lin@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
---
 .../Library/UefiDevicePathLib/DevicePathFromText.c | 32 ++++++++++++++--------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
index c66b77ba6c..63cbc9449b 100644
--- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
@@ -1140,11 +1140,14 @@ DevPathFromTextFibreEx (
                                              );
 
   FibreEx->Reserved = 0;
-  Strtoi64 (WWNStr, (UINT64 *) (&FibreEx->WWN));
-  Strtoi64 (LunStr, (UINT64 *) (&FibreEx->Lun));
-
-  *(UINT64 *) (&FibreEx->WWN) = SwapBytes64 (*(UINT64 *) (&FibreEx->WWN));
-  *(UINT64 *) (&FibreEx->Lun) = SwapBytes64 (*(UINT64 *) (&FibreEx->Lun));
+  StrHexToBytes (
+    WWNStr, sizeof (FibreEx->WWN) * 2,
+    FibreEx->WWN, sizeof (FibreEx->WWN)
+    );
+  StrHexToBytes (
+    LunStr, sizeof (FibreEx->Lun) * 2,
+    FibreEx->Lun, sizeof (FibreEx->Lun)
+    );
 
   return (EFI_DEVICE_PATH_PROTOCOL *) FibreEx;
 }
@@ -1546,8 +1549,6 @@ DevPathFromTextSasEx (
   CHAR16            *DriveBayStr;
   UINT16            Info;
   UINT16            Uint16;
-  UINT64            SasAddress;
-  UINT64            Lun;
   SASEX_DEVICE_PATH *SasEx;
 
   AddressStr  = GetNextParamStr (&TextDeviceNode);
@@ -1563,10 +1564,14 @@ DevPathFromTextSasEx (
                                         (UINT16) sizeof (SASEX_DEVICE_PATH)
                                         );
 
-  Strtoi64 (AddressStr, &SasAddress);
-  Strtoi64 (LunStr,     &Lun);
-  WriteUnaligned64 ((UINT64 *) &SasEx->SasAddress, SwapBytes64 (SasAddress));
-  WriteUnaligned64 ((UINT64 *) &SasEx->Lun,        SwapBytes64 (Lun));
+  StrHexToBytes (
+    AddressStr, sizeof (SasEx->SasAddress) * 2,
+    SasEx->SasAddress, sizeof (SasEx->SasAddress)
+    );
+  StrHexToBytes (
+    LunStr, sizeof (SasEx->Lun) * 2,
+    SasEx->Lun, sizeof (SasEx->Lun)
+    );
   SasEx->RelativeTargetPort      = (UINT16) Strtoi (RTPStr);
 
   if (StrCmp (SASSATAStr, L"NoTopology") == 0) {
@@ -2560,7 +2565,10 @@ DevPathFromTextiSCSI (
   StrToAscii (NameStr, &AsciiStr);
 
   ISCSIDevPath->TargetPortalGroupTag = (UINT16) Strtoi (PortalGroupStr);
-  Strtoi64 (LunStr, &ISCSIDevPath->Lun);
+  StrHexToBytes (
+    LunStr, sizeof (ISCSIDevPath->Lun) * 2,
+    (UINT8 *)&ISCSIDevPath->Lun, sizeof (ISCSIDevPath->Lun)
+    );
 
   Options = 0x0000;
   if (StrCmp (HeaderDigestStr, L"CRC32C") == 0) {
-- 
2.16.1.windows.1

Re: [PATCH 2/2] MdePkg/DevicePathFromText: Fix byte orders when handling 8-byte array

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Friday, March 2, 2018 7:52 PM
> To: edk2-devel@lists.01.org
> Cc: Lin, Jie <jie.lin@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: [PATCH 2/2] MdePkg/DevicePathFromText: Fix byte orders when handling 8-byte array
> 
> Per UEFI spec, FibreEx.WWN, FibreEx.Lun, SasEx.Address, SasEx.Lun
> and iSCSI.Lun are all 8-byte array with byte #0 in the left.
> It means "0102030405060708" should be converted to:
>     UINT8[8] = {01, 02, 03, 04, 05, 06, 07, 08}
> or  UINT64 = {0807060504030201}
> 
> Today's implementation wrongly uses the reversed order.
> The patch fixes this issue by using StrHexToBytes().
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jie Lin <jie.lin@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
>  .../Library/UefiDevicePathLib/DevicePathFromText.c | 32 ++++++++++++++--------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> index c66b77ba6c..63cbc9449b 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> @@ -1140,11 +1140,14 @@ DevPathFromTextFibreEx (
>                                               );
> 
>    FibreEx->Reserved = 0;
> -  Strtoi64 (WWNStr, (UINT64 *) (&FibreEx->WWN));
> -  Strtoi64 (LunStr, (UINT64 *) (&FibreEx->Lun));
> -
> -  *(UINT64 *) (&FibreEx->WWN) = SwapBytes64 (*(UINT64 *) (&FibreEx->WWN));
> -  *(UINT64 *) (&FibreEx->Lun) = SwapBytes64 (*(UINT64 *) (&FibreEx->Lun));
> +  StrHexToBytes (
> +    WWNStr, sizeof (FibreEx->WWN) * 2,
> +    FibreEx->WWN, sizeof (FibreEx->WWN)
> +    );
> +  StrHexToBytes (
> +    LunStr, sizeof (FibreEx->Lun) * 2,
> +    FibreEx->Lun, sizeof (FibreEx->Lun)
> +    );
> 
>    return (EFI_DEVICE_PATH_PROTOCOL *) FibreEx;
>  }
> @@ -1546,8 +1549,6 @@ DevPathFromTextSasEx (
>    CHAR16            *DriveBayStr;
>    UINT16            Info;
>    UINT16            Uint16;
> -  UINT64            SasAddress;
> -  UINT64            Lun;
>    SASEX_DEVICE_PATH *SasEx;
> 
>    AddressStr  = GetNextParamStr (&TextDeviceNode);
> @@ -1563,10 +1564,14 @@ DevPathFromTextSasEx (
>                                          (UINT16) sizeof (SASEX_DEVICE_PATH)
>                                          );
> 
> -  Strtoi64 (AddressStr, &SasAddress);
> -  Strtoi64 (LunStr,     &Lun);
> -  WriteUnaligned64 ((UINT64 *) &SasEx->SasAddress, SwapBytes64 (SasAddress));
> -  WriteUnaligned64 ((UINT64 *) &SasEx->Lun,        SwapBytes64 (Lun));
> +  StrHexToBytes (
> +    AddressStr, sizeof (SasEx->SasAddress) * 2,
> +    SasEx->SasAddress, sizeof (SasEx->SasAddress)
> +    );
> +  StrHexToBytes (
> +    LunStr, sizeof (SasEx->Lun) * 2,
> +    SasEx->Lun, sizeof (SasEx->Lun)
> +    );
>    SasEx->RelativeTargetPort      = (UINT16) Strtoi (RTPStr);
> 
>    if (StrCmp (SASSATAStr, L"NoTopology") == 0) {
> @@ -2560,7 +2565,10 @@ DevPathFromTextiSCSI (
>    StrToAscii (NameStr, &AsciiStr);
> 
>    ISCSIDevPath->TargetPortalGroupTag = (UINT16) Strtoi (PortalGroupStr);
> -  Strtoi64 (LunStr, &ISCSIDevPath->Lun);
> +  StrHexToBytes (
> +    LunStr, sizeof (ISCSIDevPath->Lun) * 2,
> +    (UINT8 *)&ISCSIDevPath->Lun, sizeof (ISCSIDevPath->Lun)
> +    );
> 
>    Options = 0x0000;
>    if (StrCmp (HeaderDigestStr, L"CRC32C") == 0) {
> --
> 2.16.1.windows.1

Re: [PATCH 1/2] MdePkg/DevicePathFromText: Fix bug when converting iSCSI node

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
> Sent: Friday, March 2, 2018 7:52 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 1/2] MdePkg/DevicePathFromText: Fix bug when converting iSCSI node
> 
> If protocol string is not specified, default TCP(0) should be used.
> Today's implementation wrongly sets to 1 for this case.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
>  MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> index 0459f0ab25..c66b77ba6c 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> @@ -1,7 +1,7 @@
>  /** @file
>    DevicePathFromText protocol as defined in the UEFI 2.0 specification.
> 
> -Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD License
>  which accompanies this distribution.  The full text of the license may be found at
> @@ -2581,7 +2581,7 @@ DevPathFromTextiSCSI (
> 
>    ISCSIDevPath->LoginOption      = (UINT16) Options;
> 
> -  if (StrCmp (ProtocolStr, L"TCP") == 0) {
> +  if (IS_NULL (*ProtocolStr) || (StrCmp (ProtocolStr, L"TCP") == 0)) {
>      ISCSIDevPath->NetworkProtocol = 0;
>    } else {
>      //
> --
> 2.16.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel