Hi Steve,
On 5/10/24 03:54, Steve Dickson wrote:
Hello,
On 10/4/24 2:14 PM, Charles Hedrick wrote:
While looking into a problem that turns out to be somewhere else, I
noticed that in gssd_proc.c , getpwuid is used. The context is
threaded, and I verified with strace that the thread is sharing
memory with other threads. I believe this should be changed to
getpwuid_r. Similarly the following call to getpwnam.
Is this the right place for reports on nfs-utils?
Yes... but I'm not a fan of change code, that been around
for a while, without fixing a problem... What problem does changing
getpwuid to getpwuid_r fix?
Patches are always welcome!
steved.
Yeah, getpwuid(3) and getpwnam() aren't thread safe and presumably gssd
is a service so it
could have multiple concurrent callers.
You could use domething like this ...
nfs-utils: use getpwuid_r() and getpwnam_r() in gssd From: Ian Kent
<raven@xxxxxxxxxx> gssd uses getpwuid(3) and getpwnam(3) in a pthreads
context but these functions are not thread safe. Signed-off-by: Ian Kent
<raven@xxxxxxxxxx> --- utils/gssd/gssd_proc.c | 28
+++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7
deletions(-) diff --git a/utils/gssd/gssd_proc.c
b/utils/gssd/gssd_proc.c index 2ad84c59..c718be6f 100644 ---
a/utils/gssd/gssd_proc.c +++ b/utils/gssd/gssd_proc.c @@ -489,7 +489,10
@@ success: static int change_identity(uid_t uid) { - struct passwd *pw;
+ struct passwd pw; + struct passwd *ppw; + char *pw_tmp; + long tmplen;
int res; /* drop list of supplimentary groups first */ @@ -502,15
+505,25 @@ change_identity(uid_t uid) return errno; } + tmplen =
sysconf(_SC_GETPW_R_SIZE_MAX); + if (tmplen < 0) + bufsize = 16384; + +
pw_tmp = malloc(tmplen); + if (!pw_tmp) { + printerr(0, "WARNING: unable
to allocate passwd buffer\n"); + return errno ? errno : ENOMEM; + } + /*
try to get pwent for user */ - pw = getpwuid(uid); - if (!pw) { + res =
getpwuid_r(uid, &pw, pw_tmp, tmplen, &ppw); + if (!ppw) { /* if that
doesn't work, try to get one for "nobody" */ - errno = 0; - pw =
getpwnam("nobody"); - if (!pw) { + res = getpwnam_r("nobody", &pw,
pw_tmp, tmplen, &ppw); + if (!ppw) { printerr(0, "WARNING: unable to
determine gid for uid %u\n", uid); - return errno ? errno : ENOENT; +
free(pw_tmp); + return res ? res : ENOENT; } } @@ -525,6 +538,7 @@
change_identity(uid_t uid) #else res = syscall(SYS_setresgid,
pw->pw_gid, pw->pw_gid, pw->pw_gid); #endif + free(pw_tmp); if (res !=
0) { printerr(0, "WARNING: failed to set gid to %u!\n", pw->pw_gid);
return errno;
