From 4d5111b3f1a151faf8129e38f8424898588e606d Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 2 Sep 2024 08:16:25 +0200 Subject: [PATCH] More use of getpwuid_r() directly Remove src/port/user.c, call getpwuid_r() directly. This reduces some complexity and allows better control of the error behavior. For example, the old code would in some circumstances silently truncate the result string, or produce error message strings that the caller wouldn't use. src/port/user.c used to be called src/port/thread.c and contained various portability complications to support thread-safety. These are all obsolete, and all but the user-lookup functions have already been removed. This patch completes this by also removing the user-lookup functions. Also convert src/backend/libpq/auth.c to use getpwuid_r() for thread-safety. Originally, I tried to be overly correct by using sysconf(_SC_GETPW_R_SIZE_MAX) to get the buffer size for getpwuid_r(), but that doesn't work on FreeBSD. All the OS where I could find the source code internally use 1024 as the suggested buffer size, so I just ended up hardcoding that. The previous code used BUFSIZ, which is an unrelated constant from stdio.h, so its use seemed inappropriate. Reviewed-by: Heikki Linnakangas Discussion: https://www.postgresql.org/message-id/flat/5f293da9-ceb4-4937-8e52-82c25db8e4d3%40eisentraut.org --- src/backend/libpq/auth.c | 21 +++++--- src/bin/psql/nls.mk | 3 +- src/include/port.h | 6 --- src/interfaces/libpq/fe-auth.c | 23 ++++++-- src/interfaces/libpq/fe-connect.c | 23 ++++++-- src/interfaces/libpq/nls.mk | 3 +- src/port/Makefile | 3 +- src/port/meson.build | 1 - src/port/path.c | 23 ++++++-- src/port/user.c | 89 ------------------------------- 10 files changed, 72 insertions(+), 123 deletions(-) delete mode 100644 src/port/user.c diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 2b607c5270..47e8c91606 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -1857,7 +1857,10 @@ auth_peer(hbaPort *port) uid_t uid; gid_t gid; #ifndef WIN32 + struct passwd pwbuf; struct passwd *pw; + char buf[1024]; + int rc; int ret; #endif @@ -1876,16 +1879,18 @@ auth_peer(hbaPort *port) } #ifndef WIN32 - errno = 0; /* clear errno before call */ - pw = getpwuid(uid); - if (!pw) + rc = getpwuid_r(uid, &pwbuf, buf, sizeof buf, &pw); + if (rc != 0) { - int save_errno = errno; - + errno = rc; ereport(LOG, - (errmsg("could not look up local user ID %ld: %s", - (long) uid, - save_errno ? strerror(save_errno) : _("user does not exist")))); + errmsg("could not look up local user ID %ld: %m", (long) uid)); + return STATUS_ERROR; + } + else if (!pw) + { + ereport(LOG, + errmsg("local user with ID %ld does not exist", (long) uid)); return STATUS_ERROR; } diff --git a/src/bin/psql/nls.mk b/src/bin/psql/nls.mk index 7fd8fedead..ef80163f9a 100644 --- a/src/bin/psql/nls.mk +++ b/src/bin/psql/nls.mk @@ -23,8 +23,7 @@ GETTEXT_FILES = $(FRONTEND_COMMON_GETTEXT_FILES) \ ../../common/exec.c \ ../../common/fe_memutils.c \ ../../common/username.c \ - ../../common/wait_error.c \ - ../../port/user.c + ../../common/wait_error.c GETTEXT_TRIGGERS = $(FRONTEND_COMMON_GETTEXT_TRIGGERS) \ HELP0 HELPN N_ simple_prompt simple_prompt_extended GETTEXT_FLAGS = $(FRONTEND_COMMON_GETTEXT_FLAGS) \ diff --git a/src/include/port.h b/src/include/port.h index c740005267..ba9ab0d34f 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -436,12 +436,6 @@ extern size_t strnlen(const char *str, size_t maxlen); extern char *strsep(char **stringp, const char *delim); #endif -/* port/user.c */ -#ifndef WIN32 -extern bool pg_get_user_name(uid_t user_id, char *buffer, size_t buflen); -extern bool pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen); -#endif - /* * Callers should use the qsort() macro defined below instead of calling * pg_qsort() directly. diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 5c8f404463..4904d38ce1 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -28,6 +28,7 @@ #include #include #include +#include #include /* for MAXHOSTNAMELEN on most */ #include #ifdef HAVE_SYS_UCRED_H @@ -1203,7 +1204,10 @@ pg_fe_getusername(uid_t user_id, PQExpBuffer errorMessage) char username[256 + 1]; DWORD namesize = sizeof(username); #else - char pwdbuf[BUFSIZ]; + struct passwd pwbuf; + struct passwd *pw; + char buf[1024]; + int rc; #endif #ifdef WIN32 @@ -1214,10 +1218,19 @@ pg_fe_getusername(uid_t user_id, PQExpBuffer errorMessage) "user name lookup failure: error code %lu", GetLastError()); #else - if (pg_get_user_name(user_id, pwdbuf, sizeof(pwdbuf))) - name = pwdbuf; - else if (errorMessage) - appendPQExpBuffer(errorMessage, "%s\n", pwdbuf); + rc = getpwuid_r(user_id, &pwbuf, buf, sizeof buf, &pw); + if (rc != 0) + { + errno = rc; + if (errorMessage) + libpq_append_error(errorMessage, "could not look up local user ID %ld: %m", (long) user_id); + } + else if (!pw) + { + if (errorMessage) + libpq_append_error(errorMessage, "local user with ID %ld does not exist", (long) user_id); + } + name = pw->pw_name; #endif if (name) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index ab308a0580..4cd7281b6e 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -50,6 +50,7 @@ #include #include #include +#include #endif #ifdef WIN32 @@ -7702,10 +7703,24 @@ pqGetHomeDirectory(char *buf, int bufsize) const char *home; home = getenv("HOME"); - if (home == NULL || home[0] == '\0') - return pg_get_user_home_dir(geteuid(), buf, bufsize); - strlcpy(buf, home, bufsize); - return true; + if (home && home[0]) + { + strlcpy(buf, home, bufsize); + return true; + } + else + { + struct passwd pwbuf; + struct passwd *pw; + char tmpbuf[1024]; + int rc; + + rc = getpwuid_r(geteuid(), &pwbuf, tmpbuf, sizeof tmpbuf, &pw); + if (rc != 0 || !pw) + return false; + strlcpy(buf, pw->pw_dir, bufsize); + return true; + } #else char tmppath[MAX_PATH]; diff --git a/src/interfaces/libpq/nls.mk b/src/interfaces/libpq/nls.mk index 4788088eb9..ae76126585 100644 --- a/src/interfaces/libpq/nls.mk +++ b/src/interfaces/libpq/nls.mk @@ -13,8 +13,7 @@ GETTEXT_FILES = fe-auth.c \ fe-secure-common.c \ fe-secure-gssapi.c \ fe-secure-openssl.c \ - win32.c \ - ../../port/user.c + win32.c GETTEXT_TRIGGERS = libpq_append_conn_error:2 \ libpq_append_error:2 \ libpq_gettext \ diff --git a/src/port/Makefile b/src/port/Makefile index db7c02117b..9324ec2d9f 100644 --- a/src/port/Makefile +++ b/src/port/Makefile @@ -57,8 +57,7 @@ OBJS = \ quotes.o \ snprintf.o \ strerror.o \ - tar.o \ - user.o + tar.o # libpgport.a, libpgport_shlib.a, and libpgport_srv.a contain the same files # foo.o, foo_shlib.o, and foo_srv.o are all built from foo.c diff --git a/src/port/meson.build b/src/port/meson.build index ff54b7b53e..1150966ab7 100644 --- a/src/port/meson.build +++ b/src/port/meson.build @@ -20,7 +20,6 @@ pgport_sources = [ 'snprintf.c', 'strerror.c', 'tar.c', - 'user.c', ] if host_system == 'windows' diff --git a/src/port/path.c b/src/port/path.c index 330b3f9033..de4df6cd78 100644 --- a/src/port/path.c +++ b/src/port/path.c @@ -32,6 +32,7 @@ #define near #include #else +#include #include #endif @@ -934,10 +935,24 @@ get_home_path(char *ret_path) const char *home; home = getenv("HOME"); - if (home == NULL || home[0] == '\0') - return pg_get_user_home_dir(geteuid(), ret_path, MAXPGPATH); - strlcpy(ret_path, home, MAXPGPATH); - return true; + if (home && home[0]) + { + strlcpy(ret_path, home, MAXPGPATH); + return true; + } + else + { + struct passwd pwbuf; + struct passwd *pw; + char buf[1024]; + int rc; + + rc = getpwuid_r(geteuid(), &pwbuf, buf, sizeof buf, &pw); + if (rc != 0 || !pw) + return false; + strlcpy(ret_path, pw->pw_dir, MAXPGPATH); + return true; + } #else char *tmppath; diff --git a/src/port/user.c b/src/port/user.c deleted file mode 100644 index 7444aeb64b..0000000000 --- a/src/port/user.c +++ /dev/null @@ -1,89 +0,0 @@ -/*------------------------------------------------------------------------- - * - * user.c - * - * Wrapper functions for user and home directory lookup. - * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group - * - * src/port/user.c - * - *------------------------------------------------------------------------- - */ - -#include "c.h" - -#include - -#ifndef WIN32 - -/* - * pg_get_user_name - get the name of the user with the given ID - * - * On success, the user name is returned into the buffer (of size buflen), - * and "true" is returned. On failure, a localized error message is - * returned into the buffer, and "false" is returned. - */ -bool -pg_get_user_name(uid_t user_id, char *buffer, size_t buflen) -{ - char pwdbuf[BUFSIZ]; - struct passwd pwdstr; - struct passwd *pw = NULL; - int pwerr; - - pwerr = getpwuid_r(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw); - if (pw != NULL) - { - strlcpy(buffer, pw->pw_name, buflen); - return true; - } - if (pwerr != 0) - snprintf(buffer, buflen, - _("could not look up local user ID %d: %s"), - (int) user_id, - strerror_r(pwerr, pwdbuf, sizeof(pwdbuf))); - else - snprintf(buffer, buflen, - _("local user with ID %d does not exist"), - (int) user_id); - return false; -} - -/* - * pg_get_user_home_dir - get the home directory of the user with the given ID - * - * On success, the directory path is returned into the buffer (of size buflen), - * and "true" is returned. On failure, a localized error message is - * returned into the buffer, and "false" is returned. - * - * Note that this does not incorporate the common behavior of checking - * $HOME first, since it's independent of which user_id is queried. - */ -bool -pg_get_user_home_dir(uid_t user_id, char *buffer, size_t buflen) -{ - char pwdbuf[BUFSIZ]; - struct passwd pwdstr; - struct passwd *pw = NULL; - int pwerr; - - pwerr = getpwuid_r(user_id, &pwdstr, pwdbuf, sizeof(pwdbuf), &pw); - if (pw != NULL) - { - strlcpy(buffer, pw->pw_dir, buflen); - return true; - } - if (pwerr != 0) - snprintf(buffer, buflen, - _("could not look up local user ID %d: %s"), - (int) user_id, - strerror_r(pwerr, pwdbuf, sizeof(pwdbuf))); - else - snprintf(buffer, buflen, - _("local user with ID %d does not exist"), - (int) user_id); - return false; -} - -#endif /* !WIN32 */