On Tue, 2017-06-20 at 16:07 +0200, Patrick Steinhardt wrote: > The `getpwent_r` function is a non-standard but re-entrant version of > the sdardardized `getpwent` function. Next to the benefit of being > re-entrant, though, it avoids compilation with libc implementations > that > do not provide this glibc-specific extension, for example musl libc. > > As the code is usually not run in a threaded environment, it is save > to > use the non-reentrant version, though. As such, convert code to use > `getpwent` instead to fix building with other libc implementations.
Not sure we are guaranteed that, since libsemanage has external users too, e.g. sssd among others. OTOH, getpwent_r man page says that it is not really reentrant either due to shared reading position in the stream, so maybe this doesn't matter. > > Signed-off-by: Patrick Steinhardt <[email protected]> > --- > libsemanage/src/genhomedircon.c | 22 +++++----------------- > 1 file changed, 5 insertions(+), 17 deletions(-) > > diff --git a/libsemanage/src/genhomedircon.c > b/libsemanage/src/genhomedircon.c > index e8c95ee4..f58c17ce 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -295,9 +295,8 @@ static semanage_list_t > *get_home_dirs(genhomedircon_settings_t * s) > long rbuflen; > uid_t temp, minuid = 500, maxuid = 60000; > int minuid_set = 0; > - struct passwd pwstorage, *pwbuf; > + struct passwd *pwbuf; > struct stat buf; > - int retval; > > path = semanage_findval(PATH_ETC_USERADD, "HOME", "="); > if (path && *path) { > @@ -369,7 +368,7 @@ static semanage_list_t > *get_home_dirs(genhomedircon_settings_t * s) > if (rbuf == NULL) > goto fail; > setpwent(); > - while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen, > &pwbuf)) == 0) { > + while ((pwbuf = getpwent()) != NULL) { Shouldn't you have removed rbuflen and rbuf too? They are no longer used (except to allocate and free, but never used otherwise) after this change. > if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > > maxuid) > continue; > if (!semanage_list_find(shells, pwbuf->pw_shell)) > @@ -413,7 +412,7 @@ static semanage_list_t > *get_home_dirs(genhomedircon_settings_t * s) > path = NULL; > } > > - if (retval && retval != ENOENT) { > + if (errno && errno != ENOENT) { Does getpwent() set errno to ENOENT? Not mentioned in its man page, unlike for getpwent_r(). > WARN(s->h_semanage, "Error while fetching users. " > "Returning list so far."); > } > @@ -1064,10 +1063,7 @@ static int > get_group_users(genhomedircon_settings_t * s, > long grbuflen; > char *grbuf = NULL; > struct group grstorage, *group = NULL; > - > - long prbuflen; > - char *pwbuf = NULL; > - struct passwd pwstorage, *pw = NULL; > + struct passwd *pw = NULL; > > grbuflen = sysconf(_SC_GETGR_R_SIZE_MAX); > if (grbuflen <= 0) > @@ -1104,15 +1100,8 @@ static int > get_group_users(genhomedircon_settings_t * s, > } > } > > - prbuflen = sysconf(_SC_GETPW_R_SIZE_MAX); > - if (prbuflen <= 0) > - goto cleanup; > - pwbuf = malloc(prbuflen); > - if (pwbuf == NULL) > - goto cleanup; > - > setpwent(); > - while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen, > &pw)) == 0) { > + while ((pw = getpwent()) != NULL) { > // skip users who also have this group as their > // primary group > if (lfind(pw->pw_name, group->gr_mem, &nmembers, Interestingly, this goes on to call add_user(), which calls getpwnam_r(). So if that one was ever switched back to just getpwnam(), we'd have a potential problem there. > @@ -1131,7 +1120,6 @@ static int > get_group_users(genhomedircon_settings_t * s, > retval = STATUS_SUCCESS; > cleanup: > endpwent(); > - free(pwbuf); > free(grbuf); > > return retval;
