From ad761ad2d0a63270fbd243a477dc962446a5116e Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Thu, 18 Jul 2024 12:08:06 -0700 Subject: [PATCH] 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]. --- file.c | 23 +++------- process.c | 125 ++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 99 insertions(+), 49 deletions(-) diff --git a/file.c b/file.c index 5e3b303c96..9b89f36fbe 100644 --- a/file.c +++ b/file.c @@ -3690,25 +3690,20 @@ copy_home_path(VALUE result, const char *dir) return result; } -#ifdef HAVE_PWD_H -static void * -nogvl_getpwnam(void *login) -{ - return (void *)getpwnam((const char *)login); -} -#endif - VALUE rb_home_dir_of(VALUE user, VALUE result) { #ifdef HAVE_PWD_H - struct passwd *pwPtr; + VALUE dirname = rb_getpwdirnam_for_login(user); + if (dirname == Qnil) { + rb_raise(rb_eArgError, "user %"PRIsVALUE" doesn't exist", user); + } + const char *dir = RSTRING_PTR(dirname); #else extern char *getlogin(void); const char *pwPtr = 0; const char *login; # define endpwent() ((void)0) -#endif const char *dir, *username = RSTRING_PTR(user); rb_encoding *enc = rb_enc_get(user); #if defined _WIN32 @@ -3720,21 +3715,13 @@ rb_home_dir_of(VALUE user, VALUE result) dir = username = RSTRING_PTR(rb_str_conv_enc(user, enc, fsenc)); } -#ifdef HAVE_PWD_H - pwPtr = (struct passwd *)IO_WITHOUT_GVL(nogvl_getpwnam, (void *)username); -#else if ((login = getlogin()) && strcasecmp(username, login) == 0) dir = pwPtr = getenv("HOME"); -#endif if (!pwPtr) { - endpwent(); rb_raise(rb_eArgError, "user %"PRIsVALUE" doesn't exist", user); } -#ifdef HAVE_PWD_H - dir = pwPtr->pw_dir; #endif copy_home_path(result, dir); - endpwent(); return result; } 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