On 10/4/24 10:58 PM, Ian Kent wrote:
Here we go again ...
On 5/10/24 10:47, Ian Kent wrote:
Umm, let's try that again ...
On 5/10/24 10:41, Ian Kent wrote:
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.
[PATCH} nfs-utils: use getpwuid_r() and getpwnam_r() in gssd
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>
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 2ad84c59..2a376b8f 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;
}
}
@@ -521,12 +534,13 @@ change_identity(uid_t uid)
* other threads. To bypass this, we have to call syscall()
directly.
*/
#ifdef __NR_setresgid32
- res = syscall(SYS_setresgid32, pw->pw_gid, pw->pw_gid, pw->pw_gid);
+ res = syscall(SYS_setresgid32, pw.pw_gid, pw.pw_gid, pw.pw_gid);
#else
- res = syscall(SYS_setresgid, pw->pw_gid, pw->pw_gid, pw->pw_gid);
+ 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);
+ printerr(0, "WARNING: failed to set gid to %u!\n",
pw.pw_gid);
return errno;
}
checking file utils/gssd/gssd_proc.c
Hunk #1 FAILED at 489.
Hunk #2 FAILED at 502.
Hunk #3 FAILED at 521.
3 out of 3 hunks FAILED
What branch are you applying this patch to?
Maybe it is me copying the patch over...
Try git format-patch that seems to work.
steved.
