On 10/20/24 8:46 PM, Ian Kent wrote:
On 19/10/24 20:40, Steve Dickson wrote:
On 10/18/24 7:19 PM, Ian Kent wrote:
On 18/10/24 21:10, Steve Dickson wrote:
Hey Ian,
I would like to get this into the up
coming nfs-utils release for the Bakeathon
next week. Would mind reformatting the
patch (dos2unix didn't work) and
resubmit using "git format-patch"
and "git send-email".
Sure I can do that.
I'll do it over the weekend.
I am assuming the patch is tested ;-)
It was just an example so it hasn't been tested.
I'm testing it now... it seems stable.
Right, I say I didn't test it but that means I didn't test the code in
place. I cut and pasted the bits of it into a simple program and checked
it did what it was meant to do.
I didn't feel I could claim I tested it but it should (and sounds like
it did) work ok because I could have made mistakes with this.
Ian
thanks!
steved.
It was taken from code that I have in autofs that has been in use for
many years and I'm pretty sure I saw similar code already in nfs-utils
so the code pattern is well established. Some care should be sufficient
to screw it up.
Anyway I'll post it so it can be reviewed by others that are familiar
with the code pattern.
No worries... The new release, which includes the patch is
making it through Bake-a-ton testing with flying colors!
steved.
Ian
tia,
steved.
On 10/6/24 7:53 PM, Ian Kent wrote:
On 6/10/24 20:43, Steve Dickson wrote:
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.
Opps, sorry!
I thought it was the nfs-utils repo. main branch ...
Maybe the patch has DOS carriage controls, I see that a lot myself.
If a simple dos2unix doesn't fix it I'll start checking my end and
use the
"git format-patch", "git send-email" pair.
Ian
