Re: probable big in nfs-utils

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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".

I am assuming the patch is tested ;-)

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






[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux