Caller don't need to free the buffer returned from 'EfiShellGetEnv'.
So for Shell internal code if it used 'EfiShellGetEnv' to get variable the
unfreed buffer may cause memory leak.
So this patch add a 'InternalShellGetEnv' to get environment variable to avoid
this issue.
Cc: Jaben Carsey <jaben.car...@intel.com>
Cc: Ruiyu Ni <ruiyu...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <shumin....@intel.com>
---
ShellPkg/Application/Shell/Shell.c | 41 ++++++----
ShellPkg/Application/Shell/ShellProtocol.c | 125 +++++++++++++++++++++++++++++
ShellPkg/Application/Shell/ShellProtocol.h | 30 +++++++
3 files changed, 181 insertions(+), 15 deletions(-)
diff --git a/ShellPkg/Application/Shell/Shell.c
b/ShellPkg/Application/Shell/Shell.c
index be97879..8ec4531 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1436,17 +1436,20 @@ ShellConvertVariables (
IN CONST CHAR16 *OriginalCommandLine
)
{
- CONST CHAR16 *MasterEnvList;
+ CHAR16 *MasterEnvList;
+ CHAR16 *EnvListWalker;
UINTN NewSize;
CHAR16 *NewCommandLine1;
CHAR16 *NewCommandLine2;
CHAR16 *Temp;
UINTN ItemSize;
CHAR16 *ItemTemp;
+ CHAR16 *TempVar;
SCRIPT_FILE *CurrentScriptFile;
ALIAS_LIST *AliasListNode;
ASSERT(OriginalCommandLine != NULL);
+ TempVar = NULL;
ItemSize = 0;
NewSize = StrSize(OriginalCommandLine);
@@ -1477,23 +1480,27 @@ ShellConvertVariables (
}
}
- for (MasterEnvList = EfiShellGetEnv(NULL)
- ; MasterEnvList != NULL && *MasterEnvList != CHAR_NULL //&&
*(MasterEnvList+1) != CHAR_NULL
- ; MasterEnvList += StrLen(MasterEnvList) + 1
+ MasterEnvList = InternalShellGetEnv(NULL);
+ ASSERT(MasterEnvList);
+ for (EnvListWalker = MasterEnvList
+ ; EnvListWalker != NULL && *EnvListWalker != CHAR_NULL //&&
*(MasterEnvList+1) != CHAR_NULL
+ ; EnvListWalker += StrLen(EnvListWalker) + 1
){
- if (StrSize(MasterEnvList) > ItemSize) {
- ItemSize = StrSize(MasterEnvList);
+ if (StrSize(EnvListWalker) > ItemSize) {
+ ItemSize = StrSize(EnvListWalker);
}
- for (Temp = StrStr(OriginalCommandLine, MasterEnvList)
+ for (Temp = StrStr(OriginalCommandLine, EnvListWalker)
; Temp != NULL
- ; Temp = StrStr(Temp+1, MasterEnvList)
+ ; Temp = StrStr(Temp+1, EnvListWalker)
){
//
// we need a preceding and following % and if there is space no ^
preceding (if no space ignore)
//
- if (*(Temp-1) == L'%' && *(Temp+StrLen(MasterEnvList)) == L'%' &&
+ if (*(Temp-1) == L'%' && *(Temp+StrLen(EnvListWalker)) == L'%' &&
((((Temp-OriginalCommandLine)>2) && *(Temp-2) != L'^') ||
((Temp-OriginalCommandLine)<=2))) {
- NewSize+=StrSize(EfiShellGetEnv(MasterEnvList));
+ TempVar = InternalShellGetEnv(EnvListWalker);
+ NewSize+=StrSize(TempVar);
+ SHELL_FREE_NON_NULL(TempVar);
}
}
}
@@ -1508,11 +1515,12 @@ ShellConvertVariables (
SHELL_FREE_NON_NULL(NewCommandLine1);
SHELL_FREE_NON_NULL(NewCommandLine2);
SHELL_FREE_NON_NULL(ItemTemp);
+ FreePool(MasterEnvList);
return (NULL);
}
- for (MasterEnvList = EfiShellGetEnv(NULL)
- ; MasterEnvList != NULL && *MasterEnvList != CHAR_NULL
- ; MasterEnvList += StrLen(MasterEnvList) + 1
+ for (EnvListWalker = MasterEnvList
+ ; EnvListWalker != NULL && *EnvListWalker != CHAR_NULL
+ ; EnvListWalker += StrLen(EnvListWalker) + 1
){
StrCpyS( ItemTemp,
((ItemSize+(2*sizeof(CHAR16)))/sizeof(CHAR16)),
@@ -1520,14 +1528,16 @@ ShellConvertVariables (
);
StrCatS( ItemTemp,
((ItemSize+(2*sizeof(CHAR16)))/sizeof(CHAR16)),
- MasterEnvList
+ EnvListWalker
);
StrCatS( ItemTemp,
((ItemSize+(2*sizeof(CHAR16)))/sizeof(CHAR16)),
L"%"
);
- ShellCopySearchAndReplace(NewCommandLine1, NewCommandLine2, NewSize,
ItemTemp, EfiShellGetEnv(MasterEnvList), TRUE, FALSE);
+ TempVar = InternalShellGetEnv(EnvListWalker);
+ ShellCopySearchAndReplace(NewCommandLine1, NewCommandLine2, NewSize,
ItemTemp, TempVar, TRUE, FALSE);
StrCpyS(NewCommandLine1, NewSize/sizeof(CHAR16), NewCommandLine2);
+ SHELL_FREE_NON_NULL(TempVar);
}
if (CurrentScriptFile != NULL) {
for (AliasListNode =
(ALIAS_LIST*)GetFirstNode(&CurrentScriptFile->SubstList)
@@ -1552,6 +1562,7 @@ ShellConvertVariables (
FreePool(NewCommandLine2);
FreePool(ItemTemp);
+ FreePool(MasterEnvList);
return (NewCommandLine1);
}
diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
b/ShellPkg/Application/Shell/ShellProtocol.c
index 6d5fc1f..3f2e7c3 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -2721,6 +2721,131 @@ EfiShellGetEnv(
}
/**
+ Internal function to get either a single or list of environment variables.
+
+ If name is not NULL then this function returns the current value of the
specified
+ environment variable.
+
+ If Name is NULL, then a list of all environment variable names is returned.
Each is a
+ NULL terminated string with a double NULL terminating the list.
+
+ @param Name A pointer to the environment variable name. If
+ Name is NULL, then the function will return all
+ of the defined shell environment variables. In
+ the case where multiple environment variables
are
+ being returned, each variable will be
terminated by
+ a NULL, and the list will be terminated by a
double
+ NULL.
+
+ @retval !=NULL A pointer to the returned string.
+ The returned pointer should be freed by the
caller.
+
+ @retval NULL The environment variable doesn't exist or
there are
+ no environment variables.
+**/
+
+CHAR16 *
+EFIAPI
+InternalShellGetEnv(
+ IN CONST CHAR16 *Name
+ )
+{
+ EFI_STATUS Status;
+ VOID *Buffer;
+ UINTN Size;
+ LIST_ENTRY List;
+ ENV_VAR_LIST *Node;
+ CHAR16 *CurrentWriteLocation;
+
+ Size = 0;
+ Buffer = NULL;
+
+ if (Name == NULL) {
+ //
+ // Get all our environment variables
+ //
+ InitializeListHead(&List);
+ Status = GetEnvironmentVariableList(&List);
+ if (EFI_ERROR(Status)){
+ return (NULL);
+ }
+
+ //
+ // Build the semi-colon delimited list. (2 passes)
+ //
+ for ( Node = (ENV_VAR_LIST*)GetFirstNode(&List)
+ ; !IsNull(&List, &Node->Link)
+ ; Node = (ENV_VAR_LIST*)GetNextNode(&List, &Node->Link)
+ ){
+ ASSERT(Node->Key != NULL);
+ Size += StrSize(Node->Key);
+ }
+
+ Size += 2*sizeof(CHAR16);
+
+ Buffer = AllocateZeroPool(Size);
+ if (Buffer == NULL) {
+ if (!IsListEmpty (&List)) {
+ FreeEnvironmentVariableList(&List);
+ }
+ return (NULL);
+ }
+ CurrentWriteLocation = (CHAR16*)Buffer;
+
+ for ( Node = (ENV_VAR_LIST*)GetFirstNode(&List)
+ ; !IsNull(&List, &Node->Link)
+ ; Node = (ENV_VAR_LIST*)GetNextNode(&List, &Node->Link)
+ ){
+ ASSERT(Node->Key != NULL);
+ StrCpyS( CurrentWriteLocation,
+ (Size)/sizeof(CHAR16) - (CurrentWriteLocation -
((CHAR16*)Buffer)),
+ Node->Key
+ );
+ CurrentWriteLocation += StrLen(CurrentWriteLocation) + 1;
+ }
+
+ //
+ // Free the list...
+ //
+ if (!IsListEmpty (&List)) {
+ FreeEnvironmentVariableList(&List);
+ }
+ } else {
+ //
+ // We are doing a specific environment variable
+ //
+
+ //
+ // get the size we need for this EnvVariable
+ //
+ Status = SHELL_GET_ENVIRONMENT_VARIABLE_AND_ATTRIBUTES(Name, NULL, &Size,
Buffer);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // Allocate the space and recall the get function
+ //
+ Buffer = AllocateZeroPool(Size);
+ Status = SHELL_GET_ENVIRONMENT_VARIABLE_AND_ATTRIBUTES(Name, NULL,
&Size, Buffer);
+ }
+ //
+ // we didnt get it (might not exist)
+ // free the memory if we allocated any and return NULL
+ //
+ if (EFI_ERROR(Status)) {
+ if (Buffer != NULL) {
+ FreePool(Buffer);
+ }
+ return (NULL);
+ }
+ }
+
+ //
+ // return the buffer
+ //
+ return (Buffer);
+}
+
+
+/**
Internal variable setting function. Allows for setting of the read only
variables.
@param Name Points to the NULL-terminated environment
variable name.
diff --git a/ShellPkg/Application/Shell/ShellProtocol.h
b/ShellPkg/Application/Shell/ShellProtocol.h
index 5a76389..a1e3aff 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.h
+++ b/ShellPkg/Application/Shell/ShellProtocol.h
@@ -927,6 +927,36 @@ InernalEfiShellStartMonitor(
);
/**
+ Internal function to get either a single or list of environment variables.
+
+ If name is not NULL then this function returns the current value of the
specified
+ environment variable.
+
+ If Name is NULL, then a list of all environment variable names is returned.
Each is a
+ NULL terminated string with a double NULL terminating the list.
+
+ @param Name A pointer to the environment variable name. If
+ Name is NULL, then the function will return all
+ of the defined shell environment variables. In
+ the case where multiple environment variables
are
+ being returned, each variable will be
terminated by
+ a NULL, and the list will be terminated by a
double
+ NULL.
+
+ @retval !=NULL A pointer to the returned string.
+ The returned pointer should be freed by the
caller.
+
+ @retval NULL The environment variable doesn't exist or
there are
+ no environment variables.
+**/
+CHAR16 *
+EFIAPI
+InternalShellGetEnv(
+ IN CONST CHAR16 *Name
+ );
+
+
+/**
Notification function for keystrokes.
@param[in] KeyData The key that was pressed.
--
1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
