Replace remaining getpwuid() calls with getpwuid_r()?

Started by Steve Lau6 months ago3 messages
#1Steve Lau
stevelauc@outlook.com

Hi hackers, when reading the source code, I noticed that Postgres is still using getpwuid(), which is not thread-safe since it returns a pointer to the static memory that can be overwritten by concurrent calls. Then I searched "getpwuid" in the mailing list[1]https://www.postgresql.org/search/?m=1&ln=pgsql-hackers&q=getpwuid, and there was a committed patch that refactored one such usage. So maybe we can clean up the remaining getpwuid() calls?

Then I realized that Postgres does not use threads but processes, so technically IMHO getpwuid() is safe to Postgres? But that patch mentioned that[2]/messages/by-id/5f293da9-ceb4-4937-8e52-82c25db8e4d3@eisentraut.org:

This is AFAICT the only call in the getpw*() family that needs to be dealt with. (There is also a getgrnam() call, but that is called very > early in the postmaster, before multiprocessing, so we can leave that as is.)

Which indeed confuses me. So my questions are:

1. Will the thread-safety issue of getpwuid() affect Postgres? Why?
2. If the answer to the first question is yes, should we clean up the remaining getpwuid() calls or not?

[1]: https://www.postgresql.org/search/?m=1&ln=pgsql-hackers&q=getpwuid
[2]: /messages/by-id/5f293da9-ceb4-4937-8e52-82c25db8e4d3@eisentraut.org

Best regards,
Steve.

#2Thomas Munro
thomas.munro@gmail.com
In reply to: Steve Lau (#1)
Re: Replace remaining getpwuid() calls with getpwuid_r()?

On Thu, Jul 10, 2025 at 5:10 PM Steve Lau <stevelauc@outlook.com> wrote:

Hi hackers, when reading the source code, I noticed that Postgres is still using getpwuid(), which is not thread-safe since it returns a pointer to the static memory that can be overwritten by concurrent calls. Then I searched "getpwuid" in the mailing list[1], and there was a committed patch that refactored one such usage. So maybe we can clean up the remaining getpwuid() calls?

+1, we should.

Then I realized that Postgres does not use threads but processes, so technically IMHO getpwuid() is safe to Postgres? But that patch mentioned that[2]:

We would like to use threads, and many things like this are being
changed currently.

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Steve Lau (#1)
Re: Replace remaining getpwuid() calls with getpwuid_r()?

Steve Lau <stevelauc@outlook.com> writes:

1. Will the thread-safety issue of getpwuid() affect Postgres? Why?

You have to pay attention to context. AFAICS the two extant calls
to getpwuid() are in

1. psql's exec_command_cd(). psql is not multithreaded and I'd
be surprised if there is a reason to make it so.

2. src/common/username.c's get_user_name(). This might be
problematic, but looking at the current usages I very much
doubt it. Most callers are non-multithreaded src/bin/ programs,
and the one server-side call is in single-user-mode startup,
and on top of that the call is only for geteuid() which will
not change within a given process anyway.

As Thomas mentioned, we are interested in converting the server
to multithreaded operation. But this quick survey does not
suggest that getpwuid() needs to be high on the list of things
to fix.

regards, tom lane