Re: [PATCH ima-evm-utils] evmctl: fix memory leak in get_password
From: Mimi Zohar <zohar@linux.ibm.com>
Date: 2021-08-11 14:52:10
Hi Bruno, On Tue, 2021-08-10 at 17:28 -0300, Bruno Meneguele wrote:
quoted hunk ↗ jump to hunk
The variable "password" is not freed nor returned in case get_password() succeeds. Instead of using an intermediary variable ("pwd") for returning the value, use the same "password" var. Issue found by Coverity scan tool. src/evmctl.c:2565: leaked_storage: Variable "password" going out of scope leaks the storage it points to. Signed-off-by: Bruno Meneguele <redacted> --- src/evmctl.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)diff --git a/src/evmctl.c b/src/evmctl.c index 7a6f2021aa92..b49c7910a4a7 100644 --- a/src/evmctl.c +++ b/src/evmctl.c@@ -2601,8 +2601,9 @@ static struct option opts[] = { static char *get_password(void) { struct termios flags, tmp_flags; - char *password, *pwd; + char *password; int passlen = 64; + bool err = false; password = malloc(passlen); if (!password) {@@ -2622,16 +2623,24 @@ static char *get_password(void) } printf("PEM password: "); - pwd = fgets(password, passlen, stdin); + if (fgets(password, passlen, stdin) == NULL) { + perror("fgets"); + /* we still need to restore the terminal */ + err = true; + }
From the fgets manpage: fgets() returns s on success, and NULL on error or when end of file occurs while no characters have been read.
/* restore terminal */
if (tcsetattr(fileno(stdin), TCSANOW, &flags) != 0) {
perror("tcsetattr");
+ err = true;
+ }
+
+ if (err) {
free(password);
return NULL;
}
- return pwd;
+ return password;
Wouldn't a simpler fix be to test "pwd" here?
if (!pwd)
free(password);
return pwd;
thanks,
Mimi
} int main(int argc, char *argv[])