Discussion:
[edk2] [patch] MdemodulePkg: fix the problem that data type conversion may loss data
Zhang Lubo
1970-01-01 00:00:00 UTC
Permalink
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index 9ac6363..fce80d2 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -326,15 +326,15 @@ Mtftp4SendRequest (
Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);

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 -= (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 -= (AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1);
+ Len -= (UINT32) AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
}

return Mtftp4SendPacket (Instance, Nbuf);
}
--
1.9.5.msysgit.1
Laszlo Ersek
2015-07-08 11:52:34 UTC
Permalink
Post by Zhang Lubo
Contributed-under: TianoCore Contribution Agreement 1.0
---
MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index 9ac6363..fce80d2 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -326,15 +326,15 @@ Mtftp4SendRequest (
Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);
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 -= (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 -= (AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1);
+ Len -= (UINT32) AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
}
return Mtftp4SendPacket (Instance, Nbuf);
}
I don't understand how this can avoid data loss.

I can imagine is that MSVC whines about "possible data loss", because
AsciiStrLen() returns UINTN, and Len is UINT32. However, this patch does
not prevent any truncation, it just shuts up MSVC.

Now that may be the right thing to do (I can't tell without evaluating
the function fully, which I won't volunteer for now, although I do find
it probable that the string length will be significantly smaller than
4G), but in any case, the commit message should be clear about the fact
that all the patch does is suppress the MSVC warning.

Laszlo
Ye, Ting
2015-07-09 02:12:54 UTC
Permalink
Reviewed-by: Ye Ting <***@intel.com>

-----Original Message-----
From: Zhang, Lubo
Sent: Wednesday, July 08, 2015 5:30 PM
To: edk2-***@lists.sourceforge.net; Ye, Ting; Fu, Siyuan; Hai, Yue A
Subject: [patch] MdemodulePkg: fix the problem that data type conversion may loss data

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index 9ac6363..fce80d2 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -326,15 +326,15 @@ Mtftp4SendRequest (
Len -= ((UINT32) AsciiStrLen ((CHAR8 *) Token->Filename) + (UINT32) AsciiStrLen ((CHAR8 *) Mode) + 4);

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 -= (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 -= (AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1);
+ Len -= (UINT32) AsciiStrLen ((CHAR8 *) (CHAR8 *) Options[Index].ValueStr) + 1;
}

return Mtftp4SendPacket (Instance, Nbuf);
}
--
1.9.5.msysgit.1
Loading...