Release GVL for get{pwnam,pwuid,grgid,grnam}_r calls in process.c

Do not release GVL around get{pwuid,pwnam,grgid,grnam} calls, as doing so is not thread-safe. Another C extension could have a concurrent call, and derefencing the returned pointer from these calls could result in a segfault. Have rb_home_dir_of call rb_getpwdirnam_for_login if available, so it can use getpwnam_r and release GVL in a thread-safe manner. This is related to GVL releasing work in [Bug #20587].
author: Jeremy Evans <[email protected]> 2024-07-18 12:08:06 -0700
committer: Jeremy Evans <[email protected]> 2024-09-12 07:24:02 -0700
commit: ad761ad2d0a63270fbd243a477dc962446a5116e (patch)
tree: 1d06d07e3781b77d3ba515fc9c08336b455a4480 /process.c
parent: b10500b72b2a696608015c2a2e60422a296d8614 (diff)
1 files changed, 94 insertions, 31 deletions
diff --git a/process.c b/process.c
index 88b55ed335..1326f59dd6 100644
--- a/process.c
+++ b/process.c
@@ -5802,6 +5802,26 @@ pwd_not_found(int err)
     }
 }
 
+# if defined(USE_GETPWNAM_R)
+struct getpwnam_r_args {
+    char *login;
+    char *buf;
+    size_t bufsize;
+    struct passwd *result;
+    struct passwd pwstore;
+};
+
+# define GETPWNAM_R_ARGS(login_, buf_, bufsize_) (struct getpwnam_r_args) \
+    {.login = login_, .buf = buf_, .bufsize = bufsize_, .result = NULL}
+
+static void *
+nogvl_getpwnam_r(void *args)
+{
+    struct getpwnam_r_args *arg = args;
+    return (void *)(VALUE)getpwnam_r(arg->login, &arg->pwstore, arg->buf, arg->bufsize, &arg->result);
+}
+# endif
+
 VALUE
 rb_getpwdirnam_for_login(VALUE login_name)
 {
@@ -5816,11 +5836,9 @@ rb_getpwdirnam_for_login(VALUE login_name)
 
     const char *login = RSTRING_PTR(login_name);
 
-    struct passwd *pwptr;
 
 # ifdef USE_GETPWNAM_R
 
-    struct passwd pwdnm;
     char *bufnm;
     long bufsizenm = GETPW_R_SIZE_INIT;  /* maybe -1 */
 
@@ -5832,37 +5850,39 @@ rb_getpwdirnam_for_login(VALUE login_name)
     bufnm = RSTRING_PTR(getpwnm_tmp);
     bufsizenm = rb_str_capacity(getpwnm_tmp);
     rb_str_set_len(getpwnm_tmp, bufsizenm);
+    struct getpwnam_r_args args = GETPWNAM_R_ARGS((char*)login, bufnm, (size_t)bufsizenm);
 
     int enm;
-    while ((enm = getpwnam_r(login, &pwdnm, bufnm, bufsizenm, &pwptr)) != 0) {
+    while ((enm = IO_WITHOUT_GVL_INT(nogvl_getpwnam_r, &args)) != 0) {
         if (pwd_not_found(enm)) {
             rb_str_resize(getpwnm_tmp, 0);
             return Qnil;
         }
 
-        if (enm != ERANGE || bufsizenm >= GETPW_R_SIZE_LIMIT) {
+        if (enm != ERANGE || args.bufsize >= GETPW_R_SIZE_LIMIT) {
             rb_str_resize(getpwnm_tmp, 0);
             rb_syserr_fail(enm, "getpwnam_r");
         }
 
-        rb_str_modify_expand(getpwnm_tmp, bufsizenm);
-        bufnm = RSTRING_PTR(getpwnm_tmp);
-        bufsizenm = rb_str_capacity(getpwnm_tmp);
+        rb_str_modify_expand(getpwnm_tmp, (long)args.bufsize);
+        args.buf = RSTRING_PTR(getpwnm_tmp);
+        args.bufsize = (size_t)rb_str_capacity(getpwnm_tmp);
     }
 
-    if (pwptr == NULL) {
+    if (args.result == NULL) {
         /* no record in the password database for the login name */
         rb_str_resize(getpwnm_tmp, 0);
         return Qnil;
     }
 
     /* found it */
-    VALUE result = rb_str_new_cstr(pwptr->pw_dir);
+    VALUE result = rb_str_new_cstr(args.result->pw_dir);
     rb_str_resize(getpwnm_tmp, 0);
     return result;
 
 # elif defined(USE_GETPWNAM)
 
+    struct passwd *pwptr;
     errno = 0;
     if (!(pwptr = getpwnam(login))) {
         int err = errno;
@@ -5881,6 +5901,26 @@ rb_getpwdirnam_for_login(VALUE login_name)
 #endif
 }
 
+# if defined(USE_GETPWUID_R)
+struct getpwuid_r_args {
+    uid_t uid;
+    char *buf;
+    size_t bufsize;
+    struct passwd *result;
+    struct passwd pwstore;
+};
+
+# define GETPWUID_R_ARGS(uid_, buf_, bufsize_) (struct getpwuid_r_args) \
+    {.uid = uid_, .buf = buf_, .bufsize = bufsize_, .result = NULL}
+
+static void *
+nogvl_getpwuid_r(void *args)
+{
+    struct getpwuid_r_args *arg = args;
+    return (void *)(VALUE)getpwuid_r(arg->uid, &arg->pwstore, arg->buf, arg->bufsize, &arg->result);
+}
+# endif
+
 /**
  * Look up the user's dflt home dir in the password db, by uid.
  */
@@ -5893,11 +5933,8 @@ rb_getpwdiruid(void)
 # else
     uid_t ruid = getuid();
 
-    struct passwd *pwptr;
-
 # ifdef USE_GETPWUID_R
 
-    struct passwd pwdid;
     char *bufid;
     long bufsizeid = GETPW_R_SIZE_INIT;  /* maybe -1 */
 
@@ -5909,37 +5946,39 @@ rb_getpwdiruid(void)
     bufid = RSTRING_PTR(getpwid_tmp);
     bufsizeid = rb_str_capacity(getpwid_tmp);
     rb_str_set_len(getpwid_tmp, bufsizeid);
+    struct getpwuid_r_args args = GETPWUID_R_ARGS(ruid, bufid, (size_t)bufsizeid);
 
     int eid;
-    while ((eid = getpwuid_r(ruid, &pwdid, bufid, bufsizeid, &pwptr)) != 0) {
+    while ((eid = IO_WITHOUT_GVL_INT(nogvl_getpwuid_r, &args)) != 0) {
         if (pwd_not_found(eid)) {
             rb_str_resize(getpwid_tmp, 0);
             return Qnil;
         }
 
-        if (eid != ERANGE || bufsizeid >= GETPW_R_SIZE_LIMIT) {
+        if (eid != ERANGE || args.bufsize >= GETPW_R_SIZE_LIMIT) {
             rb_str_resize(getpwid_tmp, 0);
             rb_syserr_fail(eid, "getpwuid_r");
         }
 
-        rb_str_modify_expand(getpwid_tmp, bufsizeid);
-        bufid = RSTRING_PTR(getpwid_tmp);
-        bufsizeid = rb_str_capacity(getpwid_tmp);
+        rb_str_modify_expand(getpwid_tmp, (long)args.bufsize);
+        args.buf = RSTRING_PTR(getpwid_tmp);
+        args.bufsize = (size_t)rb_str_capacity(getpwid_tmp);
     }
 
-    if (pwptr == NULL) {
+    if (args.result == NULL) {
         /* no record in the password database for the uid */
         rb_str_resize(getpwid_tmp, 0);
         return Qnil;
     }
 
     /* found it */
-    VALUE result = rb_str_new_cstr(pwptr->pw_dir);
+    VALUE result = rb_str_new_cstr(args.result->pw_dir);
     rb_str_resize(getpwid_tmp, 0);
     return result;
 
 # elif defined(USE_GETPWUID)
 
+    struct passwd *pwptr;
     errno = 0;
     if (!(pwptr = getpwuid(ruid))) {
         int err = errno;
@@ -5988,7 +6027,6 @@ obj2uid(VALUE id
         const char *usrname = StringValueCStr(id);
         struct passwd *pwptr;
 #ifdef USE_GETPWNAM_R
-        struct passwd pwbuf;
         char *getpw_buf;
         long getpw_buf_len;
         int e;
@@ -6001,15 +6039,18 @@ obj2uid(VALUE id
         getpw_buf_len = rb_str_capacity(*getpw_tmp);
         rb_str_set_len(*getpw_tmp, getpw_buf_len);
         errno = 0;
-        while ((e = getpwnam_r(usrname, &pwbuf, getpw_buf, getpw_buf_len, &pwptr)) != 0) {
-            if (e != ERANGE || getpw_buf_len >= GETPW_R_SIZE_LIMIT) {
+        struct getpwnam_r_args args = GETPWNAM_R_ARGS((char *)usrname, getpw_buf, (size_t)getpw_buf_len);
+
+        while ((e = IO_WITHOUT_GVL_INT(nogvl_getpwnam_r, &args)) != 0) {
+            if (e != ERANGE || args.bufsize >= GETPW_R_SIZE_LIMIT) {
                 rb_str_resize(*getpw_tmp, 0);
                 rb_syserr_fail(e, "getpwnam_r");
             }
-            rb_str_modify_expand(*getpw_tmp, getpw_buf_len);
-            getpw_buf = RSTRING_PTR(*getpw_tmp);
-            getpw_buf_len = rb_str_capacity(*getpw_tmp);
+            rb_str_modify_expand(*getpw_tmp, (long)args.bufsize);
+            args.buf = RSTRING_PTR(*getpw_tmp);
+            args.bufsize = (size_t)rb_str_capacity(*getpw_tmp);
         }
+        pwptr = args.result;
 #else
         pwptr = getpwnam(usrname);
 #endif
@@ -6048,6 +6089,26 @@ p_uid_from_name(VALUE self, VALUE id)
 #endif
 
 #if defined(HAVE_GRP_H)
+# if defined(USE_GETGRNAM_R)
+struct getgrnam_r_args {
+    const char *name;
+    char *buf;
+    size_t bufsize;
+    struct group *result;
+    struct group grp;
+};
+
+# define GETGRNAM_R_ARGS(name_, buf_, bufsize_) (struct getgrnam_r_args) \
+    {.name  = name_, .buf = buf_, .bufsize = bufsize_, .result = NULL}
+
+static void *
+nogvl_getgrnam_r(void *args)
+{
+    struct getgrnam_r_args *arg = args;
+    return (void *)(VALUE)getgrnam_r(arg->name, &arg->grp, arg->buf, arg->bufsize, &arg->result);
+}
+# endif
+
 static rb_gid_t
 obj2gid(VALUE id
 # ifdef USE_GETGRNAM_R
@@ -6065,7 +6126,6 @@ obj2gid(VALUE id
         const char *grpname = StringValueCStr(id);
         struct group *grptr;
 #ifdef USE_GETGRNAM_R
-        struct group grbuf;
         char *getgr_buf;
         long getgr_buf_len;
         int e;
@@ -6078,15 +6138,18 @@ obj2gid(VALUE id
         getgr_buf_len = rb_str_capacity(*getgr_tmp);
         rb_str_set_len(*getgr_tmp, getgr_buf_len);
         errno = 0;
-        while ((e = getgrnam_r(grpname, &grbuf, getgr_buf, getgr_buf_len, &grptr)) != 0) {
-            if (e != ERANGE || getgr_buf_len >= GETGR_R_SIZE_LIMIT) {
+        struct getgrnam_r_args args = GETGRNAM_R_ARGS(grpname, getgr_buf, (size_t)getgr_buf_len);
+
+        while ((e = IO_WITHOUT_GVL_INT(nogvl_getgrnam_r, &args)) != 0) {
+            if (e != ERANGE || args.bufsize >= GETGR_R_SIZE_LIMIT) {
                 rb_str_resize(*getgr_tmp, 0);
                 rb_syserr_fail(e, "getgrnam_r");
             }
-            rb_str_modify_expand(*getgr_tmp, getgr_buf_len);
-            getgr_buf = RSTRING_PTR(*getgr_tmp);
-            getgr_buf_len = rb_str_capacity(*getgr_tmp);
+            rb_str_modify_expand(*getgr_tmp, (long)args.bufsize);
+            args.buf = RSTRING_PTR(*getgr_tmp);
+            args.bufsize = (size_t)rb_str_capacity(*getgr_tmp);
         }
+        grptr = args.result;
 #elif defined(HAVE_GETGRNAM)
         grptr = getgrnam(grpname);
 #else
author	Jeremy Evans <[email protected]>	2024-07-18 12:08:06 -0700
committer	Jeremy Evans <[email protected]>	2024-09-12 07:24:02 -0700
commit	ad761ad2d0a63270fbd243a477dc962446a5116e (patch)
tree	1d06d07e3781b77d3ba515fc9c08336b455a4480 /process.c
parent	b10500b72b2a696608015c2a2e60422a296d8614 (diff)