On Sep 18, 7:31 pm, jmor...@coverity.com ("Jesse Morris") wrote: > The following bug has been logged online: > > Bug reference: 5065 > Logged by: Jesse Morris > Email address: jmor...@coverity.com > PostgreSQL version: 8.3.7, 8.4.1 > Operating system: Windows Server 2003 R2 > Description: pg_ctl start fails as administrator, with "could not > locate matching postgres executable" > Details: > > I am logged in as domain\jmorris, a member of the local Administrators > group. > ... > > From cmd.exe: > initdb.exe works fine. > pg_ctl start complains "FATAL: postgres - could not locate matching postgres > executable" > > Instrumentation and investigation reveals that the failure is in > find_other_exec (exec.c) as the error text implies, but ultimately in > pipe_read_line; CreatePipe fails with error 5 (Access Denied). ...
I went back to the version that supposedly initially fixed this issue, but I couldn't get it to work either. So I think the DACL adjustment code was always broken. The DACL stuff that both Cygwin and Active Perl use to simulate *nix file permissions masks this error, so any test framework that uses them would get false negatives on this bug. Since these DACLs are inheritable, a workaround is to run pg_ctl as a child process of Active Perl or Cygwin. The comments indicated pg_ctl & initdb were already trying to do the same thing themselves (that is, add the current user to the DACLs) but it didn't actually work on any of the systems I tried it on. I think that a number of other people have seen this bug; search for "FATAL: postgres - could not locate matching postgres executable." But that message is so misleading is probably why it seems nobody has properly diagnosed it as a permissions issue before. I didn't do anything to fix pg_ctl's error reporting. :D The patch: ------begin patch------ diff -rup unfixed/postgresql-8.4.1/src/bin/initdb/initdb.c fixed/ postgresql-8.4.1/src/bin/initdb/initdb.c --- unfixed/postgresql-8.4.1/src/bin/initdb/initdb.c 2009-06-11 07:49:07.000000000 -0700 +++ fixed/postgresql-8.4.1/src/bin/initdb/initdb.c 2009-10-15 16:31:12.651226900 -0700 @@ -2392,6 +2392,10 @@ CreateRestrictedProcess(char *cmd, PROCE fprintf(stderr, "Failed to create restricted token: %lu\n", GetLastError()); return 0; } + +#ifndef __CYGWIN__ + AddUserToTokenDacl(restrictedToken); +#endif if (!CreateProcessAsUser(restrictedToken, NULL, @@ -2409,11 +2413,7 @@ CreateRestrictedProcess(char *cmd, PROCE fprintf(stderr, "CreateProcessAsUser failed: %lu\n", GetLastError ()); return 0; } - -#ifndef __CYGWIN__ - AddUserToDacl(processInfo->hProcess); -#endif - + return ResumeThread(processInfo->hThread); } #endif Only in fixed/postgresql-8.4.1/src/bin/initdb: initdb.c.bak diff -rup unfixed/postgresql-8.4.1/src/bin/pg_ctl/pg_ctl.c fixed/ postgresql-8.4.1/src/bin/pg_ctl/pg_ctl.c --- unfixed/postgresql-8.4.1/src/bin/pg_ctl/pg_ctl.c 2009-09-01 19:40:59.000000000 -0700 +++ fixed/postgresql-8.4.1/src/bin/pg_ctl/pg_ctl.c 2009-10-15 16:31:00.096971600 -0700 @@ -1389,7 +1389,10 @@ CreateRestrictedProcess(char *cmd, PROCE write_stderr("Failed to create restricted token: %lu\n", GetLastError()); return 0; } - +#ifndef __CYGWIN__ + AddUserToTokenDacl(restrictedToken); +#endif + r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo); Kernel32Handle = LoadLibrary("KERNEL32.DLL"); @@ -1488,9 +1491,6 @@ CreateRestrictedProcess(char *cmd, PROCE } } -#ifndef __CYGWIN__ - AddUserToDacl(processInfo->hProcess); -#endif CloseHandle(restrictedToken); Only in fixed/postgresql-8.4.1/src/bin/pg_ctl: pg_ctl.c.bak diff -rup unfixed/postgresql-8.4.1/src/include/port.h fixed/ postgresql-8.4.1/src/include/port.h --- unfixed/postgresql-8.4.1/src/include/port.h 2009-06-11 07:49:08.000000000 -0700 +++ fixed/postgresql-8.4.1/src/include/port.h 2009-10-15 14:02:36.860635900 -0700 @@ -81,7 +81,7 @@ extern int find_other_exec(const char *a /* Windows security token manipulation (in exec.c) */ #ifdef WIN32 -extern BOOL AddUserToDacl(HANDLE hProcess); +extern BOOL AddUserToTokenDacl(HANDLE hToken); #endif diff -rup unfixed/postgresql-8.4.1/src/port/exec.c fixed/ postgresql-8.4.1/src/port/exec.c --- unfixed/postgresql-8.4.1/src/port/exec.c 2009-06-11 07:49:15.000000000 -0700 +++ fixed/postgresql-8.4.1/src/port/exec.c 2009-10-15 16:02:04.352805300 -0700 @@ -664,11 +664,10 @@ set_pglocale_pgservice(const char *argv0 #ifdef WIN32 /* - * AddUserToDacl(HANDLE hProcess) + * AddUserToTokenDacl(HANDLE hToken) * - * This function adds the current user account to the default DACL - * which gets attached to the restricted token used when we create - * a restricted process. + * This function adds the current user account to the restricted + * token used when we create a restricted process. * * This is required because of some security changes in Windows * that appeared in patches to XP/2K3 and in Vista/2008. @@ -681,13 +680,13 @@ set_pglocale_pgservice(const char *argv0 * and CreateProcess() calls when running as Administrator. * * This function fixes this problem by modifying the DACL of the - * specified process and explicitly re-adding the current user account. - * This is still secure because the Administrator account inherits it's - * privileges from the Administrators group - it doesn't have any of - * it's own. + * token the process will use, and explicitly re-adding the current + * user account. This is still secure because the Administrator account + * inherits it's privileges from the Administrators group - it doesn't + * have any of its own. */ BOOL -AddUserToDacl(HANDLE hProcess) +AddUserToTokenDacl(HANDLE hToken) { int i; ACL_SIZE_INFORMATION asi; @@ -695,7 +694,6 @@ AddUserToDacl(HANDLE hProcess) DWORD dwNewAclSize; DWORD dwSize = 0; DWORD dwTokenInfoLength = 0; - HANDLE hToken = NULL; PACL pacl = NULL; PSID psidUser = NULL; TOKEN_DEFAULT_DACL tddNew; @@ -703,13 +701,6 @@ AddUserToDacl(HANDLE hProcess) TOKEN_INFORMATION_CLASS tic = TokenDefaultDacl; BOOL ret = FALSE; - /* Get the token for the process */ - if (!OpenProcessToken(hProcess, TOKEN_QUERY | TOKEN_ADJUST_DEFAULT, &hToken)) - { - log_error("could not open process token: %lu", GetLastError()); - goto cleanup; - } - /* Figure out the buffer size for the DACL info */ if (!GetTokenInformation(hToken, tic, (LPVOID) NULL, dwTokenInfoLength, &dwSize)) { @@ -785,8 +776,8 @@ AddUserToDacl(HANDLE hProcess) } /* Add the new ACE for the current user */ - if (!AddAccessAllowedAce(pacl, ACL_REVISION, GENERIC_ALL, psidUser)) - { + if (!AddAccessAllowedAceEx(pacl, ACL_REVISION, OBJECT_INHERIT_ACE, GENERIC_ALL, psidUser)) + { log_error("could not add access allowed ACE: %lu", GetLastError()); goto cleanup; } @@ -812,9 +803,6 @@ cleanup: if (ptdd) LocalFree((HLOCAL) ptdd); - if (hToken) - CloseHandle(hToken); - return ret; } diff -rup unfixed/postgresql-8.4.1/src/test/regress/pg_regress.c fixed/ postgresql-8.4.1/src/test/regress/pg_regress.c --- unfixed/postgresql-8.4.1/src/test/regress/pg_regress.c 2009-06-11 07:49:15.000000000 -0700 +++ fixed/postgresql-8.4.1/src/test/regress/pg_regress.c 2009-10-15 14:03:51.609110000 -0700 @@ -1021,6 +1021,10 @@ spawn_process(const char *cmdline) cmdline2 = malloc(strlen(cmdline) + 8); sprintf(cmdline2, "cmd /c %s", cmdline); +#ifndef __CYGWIN__ + AddUserToTokenDacl(restrictedToken); +#endif + if (!CreateProcessAsUser(restrictedToken, NULL, cmdline2, @@ -1038,10 +1042,6 @@ spawn_process(const char *cmdline) exit_nicely(2); } -#ifndef __CYGWIN__ - AddUserToDacl(pi.hProcess); -#endif - free(cmdline2); ResumeThread(pi.hThread); ------end patch------ -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs