Re: probable big in nfs-utils

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

 



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.


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.


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






[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