Message ID | 1477330907-13733-7-git-send-email-ard.biesheuvel@linaro.org |
---|---|
State | Superseded |
Headers | show |
On 10/24/16 19:41, Ard Biesheuvel wrote: > Get rid of calls to unsafe string functions. These are deprecated and may > be removed in the future. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > EmbeddedPkg/Ebl/Command.c | 2 +- > EmbeddedPkg/Ebl/Dir.c | 4 ++-- > EmbeddedPkg/Ebl/EfiDevice.c | 11 ++++++----- > EmbeddedPkg/Ebl/Main.c | 8 ++++---- > EmbeddedPkg/Ebl/Variable.c | 17 +++++++++++------ > 5 files changed, 24 insertions(+), 18 deletions(-) > > diff --git a/EmbeddedPkg/Ebl/Command.c b/EmbeddedPkg/Ebl/Command.c > index e75c6a2e5c32..4bc1f4df0ca0 100644 > --- a/EmbeddedPkg/Ebl/Command.c > +++ b/EmbeddedPkg/Ebl/Command.c > @@ -614,7 +614,7 @@ OutputData ( > UINTN Spaces = 0; > CHAR8 Blanks[80]; > > - AsciiStrCpy (Blanks, mBlanks); > + AsciiStrCpyS (Blanks, sizeof Blanks, mBlanks); The original code is so poor that it makes me weep. (Not knowing who wrote this, and I don't want to look it up.) The mBlanks array has 43 (forty three) space characters in it. Please drop the mBlanks array, and use a SetMem() call here. Or... meh, I don't care. It's fine as it is. > for (EndAddress = Address + Length; Address < EndAddress; Offset += Line) { > AsciiPrint ("%08x: ", Offset); > for (Line = 0; (Line < 0x10) && (Address < EndAddress);) { > diff --git a/EmbeddedPkg/Ebl/Dir.c b/EmbeddedPkg/Ebl/Dir.c > index 36095b633019..8dd9d48ff6ac 100644 > --- a/EmbeddedPkg/Ebl/Dir.c > +++ b/EmbeddedPkg/Ebl/Dir.c > @@ -116,7 +116,7 @@ EblDirCmd ( > UnicodeFileName[0] = '\0'; > MatchSubString = &UnicodeFileName[0]; > if (Argc > 2) { > - AsciiStrToUnicodeStr (Argv[2], UnicodeFileName); > + AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE); > if (UnicodeFileName[0] == '*') { > // Handle *Name substring matching > MatchSubString = &UnicodeFileName[1]; Looks okay. > @@ -231,7 +231,7 @@ EblDirCmd ( > MatchSubString = NULL; > UnicodeFileName[0] = '\0'; > if (Argc > 2) { > - AsciiStrToUnicodeStr (Argv[2], UnicodeFileName); > + AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE); > if (UnicodeFileName[0] == '*') { > MatchSubString = &UnicodeFileName[1]; > } Ditto. > diff --git a/EmbeddedPkg/Ebl/EfiDevice.c b/EmbeddedPkg/Ebl/EfiDevice.c > index ec9c331b7004..f6969e7b2b05 100644 > --- a/EmbeddedPkg/Ebl/EfiDevice.c > +++ b/EmbeddedPkg/Ebl/EfiDevice.c > @@ -343,7 +343,7 @@ EblStartCmd ( > > ImageInfo->LoadOptionsSize = (UINT32)AsciiStrSize (Argv[2]); > ImageInfo->LoadOptions = AllocatePool (ImageInfo->LoadOptionsSize); > - AsciiStrCpy (ImageInfo->LoadOptions, Argv[2]); > + AsciiStrCpyS (ImageInfo->LoadOptions, ImageInfo->LoadOptionsSize, Argv[2]); > } > > // Transfer control to the EFI image we loaded with LoadImage() AllocateCopyPool() would be much better, but this will do. > @@ -741,7 +741,7 @@ EblFileCopyCmd ( > UINTN Size; > UINTN Offset; > UINTN Chunk = FILE_COPY_CHUNK; > - UINTN FileNameLen; > + UINTN FileNameLen, DestFileNameLen; > CHAR8* DestFileName; > CHAR8* SrcFileName; > CHAR8* SrcPtr; > @@ -786,9 +786,10 @@ EblFileCopyCmd ( > } > > // Construct the destination filepath > - DestFileName = (CHAR8*)AllocatePool (FileNameLen + AsciiStrLen (SrcFileName) + 1); > - AsciiStrCpy (DestFileName, Argv[2]); > - AsciiStrCat (DestFileName, SrcFileName); > + DestFileNameLen = FileNameLen + AsciiStrLen (SrcFileName) + 1; > + DestFileName = (CHAR8*)AllocatePool (DestFileNameLen); > + AsciiStrCpyS (DestFileName, DestFileNameLen, Argv[2]); > + AsciiStrCatS (DestFileName, DestFileNameLen, SrcFileName); > } > > Source = EfiOpen(Argv[1], EFI_FILE_MODE_READ, 0); AsciiSPrint would be (and would have been, originally) superior, but this will do. > diff --git a/EmbeddedPkg/Ebl/Main.c b/EmbeddedPkg/Ebl/Main.c > index 18b2878f69a1..4f04ae4afd33 100644 > --- a/EmbeddedPkg/Ebl/Main.c > +++ b/EmbeddedPkg/Ebl/Main.c > @@ -88,7 +88,7 @@ SetCmdHistory ( > } > > // Copy the new command line into the ring buffer > - AsciiStrnCpy(&mCmdHistory[mCmdHistoryStart][0], Cmd, MAX_CMD_LINE); > + AsciiStrnCpyS (&mCmdHistory[mCmdHistoryStart][0], MAX_CMD_LINE, Cmd, MAX_CMD_LINE); > } > > // Reset the command history for the next up arrow press > @@ -432,7 +432,7 @@ GetCmd ( > } > AsciiPrint (History); > Index = AsciiStrLen (History); > - AsciiStrnCpy (Cmd, History, CmdMaxSize); > + AsciiStrnCpyS (Cmd, CmdMaxSize, History, CmdMaxSize); > } else { > Cmd[Index++] = Char; > if (FixedPcdGetBool(PcdEmbeddedShellCharacterEcho) == TRUE) { > @@ -644,14 +644,14 @@ EdkBootLoaderEntry ( > > Status = gRT->GetVariable(CommandLineVariableName, &VendorGuid, NULL, &CommandLineVariableSize, CommandLineVariable); > if (!EFI_ERROR(Status)) { > - UnicodeStrToAsciiStr(CommandLineVariable, CmdLine); > + UnicodeStrToAsciiStrS (CommandLineVariable, CmdLine, CommandLineVariableSize); > } This is wrong, the last argument should be MAX_CMD_LINE. CommandLineVariableSize is the size (incl. the terminating NUL) of the UCS2 string read from the variable. The last argument of UnicodeStrToAsciiStrS() is DestMax, "The maximum number of Destination scii char, including terminating null char.". That is, MAX_CMD_LINE. > > FreePool(CommandLineVariable); > } > > if (EFI_ERROR(Status)) { > - AsciiStrCpy (CmdLine, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand)); > + AsciiStrCpyS (CmdLine, MAX_CMD_LINE, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand)); > } > > for (;;) { This looks good. > diff --git a/EmbeddedPkg/Ebl/Variable.c b/EmbeddedPkg/Ebl/Variable.c > index f440c48f16dd..b94531300d07 100644 > --- a/EmbeddedPkg/Ebl/Variable.c > +++ b/EmbeddedPkg/Ebl/Variable.c > @@ -29,6 +29,7 @@ EblGetCmd ( > VOID* Value; > CHAR8* AsciiVariableName = NULL; > CHAR16* VariableName; > + UINTN VariableNameLen; > UINT32 Index; > > if (Argc == 1) { > @@ -48,8 +49,9 @@ EblGetCmd ( > AsciiPrint("Variable name is missing.\n"); > return Status; > } else { > - VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16)); > - AsciiStrToUnicodeStr (AsciiVariableName,VariableName); > + VariableNameLen = (AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16); > + VariableName = AllocatePool(VariableNameLen); > + AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen); > } > > // Try to get the variable size. This is wrong. VariableNameLen is the size in bytes of a CHAR16 array (incl. the terminating L'\0'). The last argument of AsciiStrToUnicodeStrS(), is DestMax: "The maximum number of Destination Unicode char, including terminating null char." > @@ -93,6 +95,7 @@ EblSetCmd ( > CHAR8* AsciiValue; > UINT32 AsciiValueLength; > CHAR16* VariableName; > + UINTN VariableNameLen; > UINT32 Index; > UINT32 EscapedQuotes = 0; > BOOLEAN Volatile = FALSE; > @@ -125,8 +128,9 @@ EblSetCmd ( > // > > // Convert VariableName into Unicode > - VariableName = AllocatePool((AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16)); > - AsciiStrToUnicodeStr (AsciiVariableSetting,VariableName); > + VariableNameLen = (AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16); > + VariableName = AllocatePool(VariableNameLen); > + AsciiStrToUnicodeStrS (AsciiVariableSetting, VariableName, VariableNameLen); > > Status = gRT->SetVariable ( > VariableName, Same issue here. > @@ -170,8 +174,9 @@ EblSetCmd ( > } > > // Convert VariableName into Unicode > - VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16)); > - AsciiStrToUnicodeStr (AsciiVariableName,VariableName); > + VariableNameLen = (AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16); > + VariableName = AllocatePool(VariableNameLen); > + AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen); > > Status = gRT->SetVariable ( > VariableName, > And here. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/EmbeddedPkg/Ebl/Command.c b/EmbeddedPkg/Ebl/Command.c index e75c6a2e5c32..4bc1f4df0ca0 100644 --- a/EmbeddedPkg/Ebl/Command.c +++ b/EmbeddedPkg/Ebl/Command.c @@ -614,7 +614,7 @@ OutputData ( UINTN Spaces = 0; CHAR8 Blanks[80]; - AsciiStrCpy (Blanks, mBlanks); + AsciiStrCpyS (Blanks, sizeof Blanks, mBlanks); for (EndAddress = Address + Length; Address < EndAddress; Offset += Line) { AsciiPrint ("%08x: ", Offset); for (Line = 0; (Line < 0x10) && (Address < EndAddress);) { diff --git a/EmbeddedPkg/Ebl/Dir.c b/EmbeddedPkg/Ebl/Dir.c index 36095b633019..8dd9d48ff6ac 100644 --- a/EmbeddedPkg/Ebl/Dir.c +++ b/EmbeddedPkg/Ebl/Dir.c @@ -116,7 +116,7 @@ EblDirCmd ( UnicodeFileName[0] = '\0'; MatchSubString = &UnicodeFileName[0]; if (Argc > 2) { - AsciiStrToUnicodeStr (Argv[2], UnicodeFileName); + AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE); if (UnicodeFileName[0] == '*') { // Handle *Name substring matching MatchSubString = &UnicodeFileName[1]; @@ -231,7 +231,7 @@ EblDirCmd ( MatchSubString = NULL; UnicodeFileName[0] = '\0'; if (Argc > 2) { - AsciiStrToUnicodeStr (Argv[2], UnicodeFileName); + AsciiStrToUnicodeStrS (Argv[2], UnicodeFileName, MAX_CMD_LINE); if (UnicodeFileName[0] == '*') { MatchSubString = &UnicodeFileName[1]; } diff --git a/EmbeddedPkg/Ebl/EfiDevice.c b/EmbeddedPkg/Ebl/EfiDevice.c index ec9c331b7004..f6969e7b2b05 100644 --- a/EmbeddedPkg/Ebl/EfiDevice.c +++ b/EmbeddedPkg/Ebl/EfiDevice.c @@ -343,7 +343,7 @@ EblStartCmd ( ImageInfo->LoadOptionsSize = (UINT32)AsciiStrSize (Argv[2]); ImageInfo->LoadOptions = AllocatePool (ImageInfo->LoadOptionsSize); - AsciiStrCpy (ImageInfo->LoadOptions, Argv[2]); + AsciiStrCpyS (ImageInfo->LoadOptions, ImageInfo->LoadOptionsSize, Argv[2]); } // Transfer control to the EFI image we loaded with LoadImage() @@ -741,7 +741,7 @@ EblFileCopyCmd ( UINTN Size; UINTN Offset; UINTN Chunk = FILE_COPY_CHUNK; - UINTN FileNameLen; + UINTN FileNameLen, DestFileNameLen; CHAR8* DestFileName; CHAR8* SrcFileName; CHAR8* SrcPtr; @@ -786,9 +786,10 @@ EblFileCopyCmd ( } // Construct the destination filepath - DestFileName = (CHAR8*)AllocatePool (FileNameLen + AsciiStrLen (SrcFileName) + 1); - AsciiStrCpy (DestFileName, Argv[2]); - AsciiStrCat (DestFileName, SrcFileName); + DestFileNameLen = FileNameLen + AsciiStrLen (SrcFileName) + 1; + DestFileName = (CHAR8*)AllocatePool (DestFileNameLen); + AsciiStrCpyS (DestFileName, DestFileNameLen, Argv[2]); + AsciiStrCatS (DestFileName, DestFileNameLen, SrcFileName); } Source = EfiOpen(Argv[1], EFI_FILE_MODE_READ, 0); diff --git a/EmbeddedPkg/Ebl/Main.c b/EmbeddedPkg/Ebl/Main.c index 18b2878f69a1..4f04ae4afd33 100644 --- a/EmbeddedPkg/Ebl/Main.c +++ b/EmbeddedPkg/Ebl/Main.c @@ -88,7 +88,7 @@ SetCmdHistory ( } // Copy the new command line into the ring buffer - AsciiStrnCpy(&mCmdHistory[mCmdHistoryStart][0], Cmd, MAX_CMD_LINE); + AsciiStrnCpyS (&mCmdHistory[mCmdHistoryStart][0], MAX_CMD_LINE, Cmd, MAX_CMD_LINE); } // Reset the command history for the next up arrow press @@ -432,7 +432,7 @@ GetCmd ( } AsciiPrint (History); Index = AsciiStrLen (History); - AsciiStrnCpy (Cmd, History, CmdMaxSize); + AsciiStrnCpyS (Cmd, CmdMaxSize, History, CmdMaxSize); } else { Cmd[Index++] = Char; if (FixedPcdGetBool(PcdEmbeddedShellCharacterEcho) == TRUE) { @@ -644,14 +644,14 @@ EdkBootLoaderEntry ( Status = gRT->GetVariable(CommandLineVariableName, &VendorGuid, NULL, &CommandLineVariableSize, CommandLineVariable); if (!EFI_ERROR(Status)) { - UnicodeStrToAsciiStr(CommandLineVariable, CmdLine); + UnicodeStrToAsciiStrS (CommandLineVariable, CmdLine, CommandLineVariableSize); } FreePool(CommandLineVariable); } if (EFI_ERROR(Status)) { - AsciiStrCpy (CmdLine, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand)); + AsciiStrCpyS (CmdLine, MAX_CMD_LINE, (CHAR8 *)PcdGetPtr (PcdEmbeddedAutomaticBootCommand)); } for (;;) { diff --git a/EmbeddedPkg/Ebl/Variable.c b/EmbeddedPkg/Ebl/Variable.c index f440c48f16dd..b94531300d07 100644 --- a/EmbeddedPkg/Ebl/Variable.c +++ b/EmbeddedPkg/Ebl/Variable.c @@ -29,6 +29,7 @@ EblGetCmd ( VOID* Value; CHAR8* AsciiVariableName = NULL; CHAR16* VariableName; + UINTN VariableNameLen; UINT32 Index; if (Argc == 1) { @@ -48,8 +49,9 @@ EblGetCmd ( AsciiPrint("Variable name is missing.\n"); return Status; } else { - VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16)); - AsciiStrToUnicodeStr (AsciiVariableName,VariableName); + VariableNameLen = (AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16); + VariableName = AllocatePool(VariableNameLen); + AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen); } // Try to get the variable size. @@ -93,6 +95,7 @@ EblSetCmd ( CHAR8* AsciiValue; UINT32 AsciiValueLength; CHAR16* VariableName; + UINTN VariableNameLen; UINT32 Index; UINT32 EscapedQuotes = 0; BOOLEAN Volatile = FALSE; @@ -125,8 +128,9 @@ EblSetCmd ( // // Convert VariableName into Unicode - VariableName = AllocatePool((AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16)); - AsciiStrToUnicodeStr (AsciiVariableSetting,VariableName); + VariableNameLen = (AsciiStrLen (AsciiVariableSetting) + 1) * sizeof (CHAR16); + VariableName = AllocatePool(VariableNameLen); + AsciiStrToUnicodeStrS (AsciiVariableSetting, VariableName, VariableNameLen); Status = gRT->SetVariable ( VariableName, @@ -170,8 +174,9 @@ EblSetCmd ( } // Convert VariableName into Unicode - VariableName = AllocatePool((AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16)); - AsciiStrToUnicodeStr (AsciiVariableName,VariableName); + VariableNameLen = (AsciiStrLen (AsciiVariableName) + 1) * sizeof (CHAR16); + VariableName = AllocatePool(VariableNameLen); + AsciiStrToUnicodeStrS (AsciiVariableName, VariableName, VariableNameLen); Status = gRT->SetVariable ( VariableName,
Get rid of calls to unsafe string functions. These are deprecated and may be removed in the future. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- EmbeddedPkg/Ebl/Command.c | 2 +- EmbeddedPkg/Ebl/Dir.c | 4 ++-- EmbeddedPkg/Ebl/EfiDevice.c | 11 ++++++----- EmbeddedPkg/Ebl/Main.c | 8 ++++---- EmbeddedPkg/Ebl/Variable.c | 17 +++++++++++------ 5 files changed, 24 insertions(+), 18 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel