Fix some memory leaks and improve restricted token handling on Windows

The leaks have been detected by a Coverity run on Windows.  No backpatch
is done as the leaks are minor.

While on it, make restricted token creation more consistent in its error
handling by logging an error instead of a warning if missing
advapi32.dll, which was missing in the NT4 days.  Any modern platform
should have this DLL around.  Now, if the library is not there, an error
is still reported back to the caller, and nothing is done do there is no
behavior change done in this commit.

Author: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQApa9MG0foPkgPX87fipk=vhnF2Xfg+CfUyR08h4R7Mywg@mail.gmail.com

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 511f8913939b45f9e84121972c3bed84547191a9..bd8c7f58119452a552f66ddfb4e3838235099aa1 100644 (file)
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -1387,6 +1387,13 @@ pg_SSPI_recvauth(Port *port)
        mtype = pq_getbyte();
        if (mtype != 'p')
        {
+           if (sspictx != NULL)
+           {
+               DeleteSecurityContext(sspictx);
+               free(sspictx);
+           }
+           FreeCredentialsHandle(&sspicred);
+
            /* Only log error if client didn't disconnect. */
            if (mtype != EOF)
                ereport(ERROR,
@@ -1402,6 +1409,12 @@ pg_SSPI_recvauth(Port *port)
        {
            /* EOF - pq_getmessage already logged error */
            pfree(buf.data);
+           if (sspictx != NULL)
+           {
+               DeleteSecurityContext(sspictx);
+               free(sspictx);
+           }
+           FreeCredentialsHandle(&sspicred);
            return STATUS_ERROR;
        }
 
@@ -2517,6 +2530,7 @@ InitializeLDAPConnection(Port *port, LDAP **ldap)
                        (errmsg("could not load function _ldap_start_tls_sA in wldap32.dll"),
                         errdetail("LDAP over SSL is not supported on this platform.")));
                ldap_unbind(*ldap);
+               FreeLibrary(ldaphandle);
                return STATUS_ERROR;
            }
 

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 7a92dac52556a7d82cd57facdd664442a010b6f2..b3986bee75fe643d42376694b3242dcc5bce0a1d 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4719,6 +4719,8 @@ retry:
    if (cmdLine[sizeof(cmdLine) - 2] != '\0')
    {
        elog(LOG, "subprocess command line too long");
+       UnmapViewOfFile(param);
+       CloseHandle(paramHandle);
        return -1;
    }
 
@@ -4735,6 +4737,8 @@ retry:
    {
        elog(LOG, "CreateProcess call failed: %m (error code %lu)",
             GetLastError());
+       UnmapViewOfFile(param);
+       CloseHandle(paramHandle);
        return -1;
    }
 
@@ -4750,6 +4754,8 @@ retry:
                                     GetLastError())));
        CloseHandle(pi.hProcess);
        CloseHandle(pi.hThread);
+       UnmapViewOfFile(param);
+       CloseHandle(paramHandle);
        return -1;              /* log made by save_backend_variables */
    }
 

diff --git a/src/common/restricted_token.c b/src/common/restricted_token.c
index 74ba7192a16b753b61c38fa1d6cf4814122dd1b8..a3e0e85fefac5c086dbda2b4cb24f4c2a1ca460c 100644 (file)
--- a/src/common/restricted_token.c
+++ b/src/common/restricted_token.c
@@ -40,8 +40,8 @@ typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_
  *
  * Returns restricted token on success and 0 on failure.
  *
- * On NT4, or any other system not containing the required functions, will
- * NOT execute anything.
+ * On any system not containing the required functions, do nothing
+ * but still report an error.
  */
 HANDLE
 CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
@@ -52,30 +52,36 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
    HANDLE      restrictedToken;
    SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
    SID_AND_ATTRIBUTES dropSids[2];
-   __CreateRestrictedToken _CreateRestrictedToken = NULL;
+   __CreateRestrictedToken _CreateRestrictedToken;
    HANDLE      Advapi32Handle;
 
    ZeroMemory(&si, sizeof(si));
    si.cb = sizeof(si);
 
    Advapi32Handle = LoadLibrary("ADVAPI32.DLL");
-   if (Advapi32Handle != NULL)
+   if (Advapi32Handle == NULL)
    {
-       _CreateRestrictedToken = (__CreateRestrictedToken) GetProcAddress(Advapi32Handle, "CreateRestrictedToken");
+       pg_log_error("could not load advapi32.dll: error code %lu",
+                    GetLastError());
+       return 0;
    }
 
+   _CreateRestrictedToken = (__CreateRestrictedToken) GetProcAddress(Advapi32Handle, "CreateRestrictedToken");
+
    if (_CreateRestrictedToken == NULL)
    {
-       pg_log_warning("cannot create restricted tokens on this platform");
-       if (Advapi32Handle != NULL)
-           FreeLibrary(Advapi32Handle);
+       pg_log_error("cannot create restricted tokens on this platform: error code %lu",
+                    GetLastError());
+       FreeLibrary(Advapi32Handle);
        return 0;
    }
 
    /* Open the current token to use as a base for the restricted one */
    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken))
    {
-       pg_log_error("could not open process token: error code %lu", GetLastError());
+       pg_log_error("could not open process token: error code %lu",
+                    GetLastError());
+       FreeLibrary(Advapi32Handle);
        return 0;
    }
 
@@ -88,7 +94,10 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo)
                                  SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_POWER_USERS, 0, 0, 0, 0, 0,
                                  0, &dropSids[1].Sid))
    {
-       pg_log_error("could not allocate SIDs: error code %lu", GetLastError());
+       pg_log_error("could not allocate SIDs: error code %lu",
+                    GetLastError());
+       CloseHandle(origToken);
+       FreeLibrary(Advapi32Handle);
        return 0;
    }
 
@@ -171,8 +180,8 @@ get_restricted_token(void)
        else
        {
            /*
-            * Successfully re-execed. Now wait for child process to capture
-            * exitcode.
+            * Successfully re-executed. Now wait for child process to capture
+            * the exit code.
             */
            DWORD       x;
 
@@ -187,6 +196,7 @@ get_restricted_token(void)
            }
            exit(x);
        }
+       pg_free(cmdline);
    }
 #endif
 }