Hi, Lubo
For Ifconfig6.c, it's not a correct fix, I guess the original code want to
check whether String is NULL or not. Simply remove it is not safe.
Mtftp6Support patch has the same issue as I said in Mtftp4Support. And since
you always allocate length enough buffer, the AsciiStrCpyS should not return an
error so you should use ASSERT_EFI_ERROR instead of check the return code of it.
Siyuan
-----Original Message-----
From: Zhang, Lubo
Sent: Thursday, July 09, 2015 4:04 PM
To: edk2-devel@lists.sourceforge.net; Qiu, Shumin; Fu, Siyuan
Subject: [patch] NetworkPkg: Fix a bug that return type differs from the left
one when assigned, another is redefinition when checking whether the string is
null.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zh...@intel.com>
---
NetworkPkg/Application/IfConfig6/IfConfig6.c | 2 +-
NetworkPkg/Mtftp6Dxe/Mtftp6Support.c | 47 ++++++++++++++++++++++------
2 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/NetworkPkg/Application/IfConfig6/IfConfig6.c
b/NetworkPkg/Application/IfConfig6/IfConfig6.c
index 4cec44f..e66d52a 100644
--- a/NetworkPkg/Application/IfConfig6/IfConfig6.c
+++ b/NetworkPkg/Application/IfConfig6/IfConfig6.c
@@ -125,11 +125,11 @@ SplitStrToList (
CHAR16 *Str;
CHAR16 *ArgStr;
ARG_LIST *ArgList;
ARG_LIST *ArgNode;
- if (*String == L'\0' || *String == NULL) {
+ if (*String == L'\0') {
return NULL;
}
//
// Copy the CONST string to a local copy.
diff --git a/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
b/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
index 91680b2..fc8a469 100644
--- a/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
+++ b/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
@@ -450,10 +450,11 @@ Mtftp6SendRequest (
)
{
EFI_MTFTP6_PACKET *Packet;
EFI_MTFTP6_OPTION *Options;
EFI_MTFTP6_TOKEN *Token;
+ RETURN_STATUS Status;
NET_BUF *Nbuf;
UINT8 *Mode;
UINT8 *Cur;
UINT32 Len1;
UINT32 Len2;
@@ -510,26 +511,50 @@ Mtftp6SendRequest (
Packet = (EFI_MTFTP6_PACKET *) NetbufAllocSpace (Nbuf, Len, FALSE);
ASSERT (Packet != NULL);
Packet->OpCode = HTONS (Operation);
Cur = Packet->Rrq.Filename;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2, (CHAR8 *)
Token->Filename);
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, Len - 2, (CHAR8 *)
Token->Filename);
+
+ if (EFI_ERROR (Status)) {
+ goto ERROR_EXIT;
+ }
+
+ Cur = (UINT8 *) Cur;
Cur += AsciiStrLen ((CHAR8 *) Token->Filename) + 1;
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len - 2 -
(AsciiStrLen ((CHAR8 *) Token->Filename) + 1), (CHAR8 *) Mode);
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, Len - 2 - (AsciiStrLen ((CHAR8
*) Token->Filename) + 1), (CHAR8 *) Mode);
+
+ if (EFI_ERROR (Status)) {
+ goto ERROR_EXIT;
+ }
+
+ Cur = (UINT8 *) Cur;
Cur += AsciiStrLen ((CHAR8 *) Mode) + 1;
- Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32)
AsciiStrLen ((CHAR8 *) Mode) + 4);
+ Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) +
(UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);
//
// Copy all the extension options into the packet.
//
for (Index = 0; Index < Token->OptionCount; ++Index) {
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *)
Options[Index].OptionStr);
- Cur += AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
- Len -= (UINT32)(AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1);
- Cur = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *)
Options[Index].ValueStr);
- Cur += AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
- Len -= (UINT32)(AsciiStrLen ((CHAR8 *) Options[Index].ValueStr) + 1);
+ Status = AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *)
Options[Index].OptionStr);
+
+ if (EFI_ERROR (Status)) {
+ goto ERROR_EXIT;
+ }
+
+ Cur = (UINT8 *) Cur;
+ Cur += AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1;
+ Len -= (UINT32)(AsciiStrLen ((CHAR8 *) Options[Index].OptionStr) + 1);
+ Status = (UINT8 *) AsciiStrCpyS ((CHAR8 *) Cur, Len, (CHAR8 *)
Options[Index].ValueStr);
+
+ if (EFI_ERROR (Status)) {
+ goto ERROR_EXIT;
+ }
+
+ Cur = (UINT8 *) Cur;
+ Cur += AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
+ Len -= (UINT32)(AsciiStrLen ((CHAR8 *) Options[Index].ValueStr) + 1);
}
//
// Save the packet buf for retransmit
//
@@ -539,10 +564,14 @@ Mtftp6SendRequest (
Instance->LastPacket = Nbuf;
Instance->CurRetry = 0;
return Mtftp6TransmitPacket (Instance, Nbuf);
+
+ERROR_EXIT:
+ NetbufFree (Nbuf);
+ return Status;
}
/**
Build and send an error packet.
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
