Portability improvements
cover Portability improvements
Hi,
This is a small batch of portability fixes. It removes calls to the non-portable getpwent_r function, problems with hardened toolchains defining FORTIFY_SOURCE and removes usage of __BEGIN_DECLS. These patches are all aiming for compatibility with musl on hardened Gentoo.
While I guess that the first two commits of removing __BEGIN_DECLS and the patch for FORTIFY_SOURCE are uncontroversial, I can imagine the removal of getpwent_r being a bit more so due to the alternative being non-reentrant. But to the best of my knowledge the code in question is not called in a threaded context, so I actually doubt it matters much. Given that I'm new to this code base I could very likely be wrong, though.
Patrick
Patrick Steinhardt (3):
libsepol: replace non-standard use of __BEGIN_DECLS
libselinux: fix error when FORTIFY_SOURCE is already set
genhomedircon: avoid use of non-standard getpwent_r
libselinux/src/Makefile | 3 ++- libselinux/utils/Makefile | 3 ++- libsemanage/src/genhomedircon.c | 22 +++++----------------- libsepol/include/sepol/ibendport_record.h | 10 +++++++--- libsepol/include/sepol/ibendports.h | 11 ++++++++--- libsepol/include/sepol/ibpkey_record.h | 11 ++++++++--- libsepol/include/sepol/ibpkeys.h | 12 +++++++++--- 7 files changed, 41 insertions(+), 31 deletions(-)
Signature · 1 line
2.13.1
PATCH 1/3 libsepol: replace non-standard use of __BEGIN_DECLS
While most header files already use the common pattern of extern "C" declarations to enable compiling in a C++ project, some header files in libsepol instead use the macros __BEGIN_DECLS and __END_DECLS. These macros are defined in the "sys/cdefs.h" header file, which provides some non-standard extensions for glibc.
Convert usage of these declarations with the standard extern "C" pattern. This improves compatibility with other libc implementations, e.g. musl libc.
--- libsepol/include/sepol/ibendport_record.h | 10 +++++++--- libsepol/include/sepol/ibendports.h | 11 ++++++++--- libsepol/include/sepol/ibpkey_record.h | 11 ++++++++--- libsepol/include/sepol/ibpkeys.h | 12 +++++++++--- 4 files changed, 32 insertions(+), 12 deletions(-)
libsepol/include/sepol/ibendport_record.h +7 -3
@@ -4,9 +4,10 @@#include <stddef.h> #include <sepol/context_record.h> #include <sepol/handle.h> -#include <sys/cdefs.h> -__BEGIN_DECLS +#ifdef __cplusplus +extern "C" { +#endif struct sepol_ibendport ;struct sepol_ibendport_key ;
@@ -64,5 +65,8 @@ extern int sepol_ibendport_clone(sepol_handle_t *handle,extern void sepol_ibendport_free (sepol_ibendport_t *ibendport); -__END_DECLS +#ifdef __cplusplus +} +#endif +#endif
libsepol/include/sepol/ibendports.h +8 -3
@@ -4,9 +4,10 @@#include <sepol/handle.h> #include <sepol/policydb.h> #include <sepol/ibendport_record.h> -#include <sys/cdefs.h> -__BEGIN_DECLS +#ifdef __cplusplus +extern "C" { +#endif /* Return the number of ibendports */ extern int sepol_ibendport_count(sepol_handle_t *handle,
@@ -41,5 +42,9 @@ extern int sepol_ibendport_iterate(sepol_handle_t *handle,int (*fn)(const sepol_ibendport_t *ibendport, void *fn_arg), void *arg); -__END_DECLS + +#ifdef __cplusplus +} +#endif +#endif
libsepol/include/sepol/ibpkey_record.h +8 -3
@@ -5,11 +5,12 @@#include <stdint.h> #include <sepol/context_record.h> #include <sepol/handle.h> -#include <sys/cdefs.h> #define INET6_ADDRLEN 16 -__BEGIN_DECLS +#ifdef __cplusplus +extern "C" { +#endif struct sepol_ibpkey ;struct sepol_ibpkey_key ;
@@ -71,5 +72,9 @@ extern int sepol_ibpkey_clone(sepol_handle_t *handle,extern void sepol_ibpkey_free (sepol_ibpkey_t *ibpkey); -__END_DECLS + +#ifdef __cplusplus +} +#endif +#endif
libsepol/include/sepol/ibpkeys.h +9 -3
@@ -4,9 +4,11 @@#include <sepol/handle.h> #include <sepol/policydb.h> #include <sepol/ibpkey_record.h> -#include <sys/cdefs.h> -__BEGIN_DECLS + +#ifdef __cplusplus +extern "C" { +#endif /* Return the number of ibpkeys */ extern int sepol_ibpkey_count(sepol_handle_t *handle,
@@ -40,5 +42,9 @@ extern int sepol_ibpkey_iterate(sepol_handle_t *handle,int (*fn)(const sepol_ibpkey_t *ibpkey, void *fn_arg), void *arg); -__END_DECLS + +#ifdef __cplusplus +} +#endif +#endif
Signature · 1 line
2.13.1
Stephen Smalley
AppliedRe: [PATCH 1/3] libsepol: replace non-standard use of __BEGIN_DECLS
Quoted 9 lines · Patrick Steinhardt
While most header files already use the common pattern of
extern "C"
declarations to enable compiling in a C++ project, some header files in
libsepol instead use the macros__BEGIN_DECLSand__END_DECLS. These
macros are defined in the "sys/cdefs.h" header file, which provides some non-standard extensions for glibc.
Quoted 5 lines
Convert usage of these declarations with the standard
extern "C"pattern. This improves compatibility with other libc implementations, e.g. musl libc.Signed-off-by Patrick Steinhardt <[email protected]>
Thanks, applied.
Quoted 132 lines
---
libsepol/include/sepol/ibendport_record.h | 10 +++++++--- libsepol/include/sepol/ibendports.h | 11 ++++++++--- libsepol/include/sepol/ibpkey_record.h | 11 ++++++++--- libsepol/include/sepol/ibpkeys.h | 12 +++++++++---4 files changed, 32 insertions(+), 12 deletions(-)libsepol/include/sepol/ibendport_record.h +0
-1@@ -4,9 +4,10 @@ #include <stddef.h> #include <sepol/context_record.h> #include <sepol/handle.h> -#include <sys/cdefs.h> libsepol/include/sepol/ibendport_record.h +3
-1-__BEGIN_DECLS +#ifdef __cplusplus +extern "C" { +#endif
structsepol_ibendport;
structsepol_ibendport_key;libsepol/include/sepol/ibendport_record.h +0
-0@@ -64,5 +65,8 @@ extern int sepol_ibendport_clone(sepol_handle_t *handle,
extern voidsepol_ibendport_free(sepol_ibendport_t *ibendport);libsepol/include/sepol/ibendport_record.h +4
-1-__END_DECLS +#ifdef __cplusplus +} +#endif +#endif
libsepol/include/sepol/ibendports.h +0
-1@@ -4,9 +4,10 @@ #include <sepol/handle.h> #include <sepol/policydb.h> #include <sepol/ibendport_record.h> -#include <sys/cdefs.h> libsepol/include/sepol/ibendports.h +3
-1-__BEGIN_DECLS +#ifdef __cplusplus +extern "C" { +#endif
/* Return the number of ibendports */
extern int sepol_ibendport_count(sepol_handle_t *handle,libsepol/include/sepol/ibendports.h +0
-0@@ -41,5 +42,9 @@ extern int sepol_ibendport_iterate(sepol_handle_t *handle,int (*fn)(const
sepol_ibendport_t*ibendport,
void*fn_arg), void *arg);libsepol/include/sepol/ibendports.h +5
-1-__END_DECLS + +#ifdef __cplusplus +} +#endif +#endif
libsepol/include/sepol/ibpkey_record.h +0
-1@@ -5,11 +5,12 @@ #include <stdint.h> #include <sepol/context_record.h> #include <sepol/handle.h> -#include <sys/cdefs.h>
#defineINET6_ADDRLEN16libsepol/include/sepol/ibpkey_record.h +3
-1-__BEGIN_DECLS +#ifdef __cplusplus +extern "C" { +#endif
structsepol_ibpkey;
structsepol_ibpkey_key;libsepol/include/sepol/ibpkey_record.h +0
-0@@ -71,5 +72,9 @@ extern int sepol_ibpkey_clone(sepol_handle_t *handle,
extern voidsepol_ibpkey_free(sepol_ibpkey_t *ibpkey);libsepol/include/sepol/ibpkey_record.h +5
-1-__END_DECLS + +#ifdef __cplusplus +} +#endif +#endif
libsepol/include/sepol/ibpkeys.h +0
-1@@ -4,9 +4,11 @@ #include <sepol/handle.h> #include <sepol/policydb.h> #include <sepol/ibpkey_record.h> -#include <sys/cdefs.h> libsepol/include/sepol/ibpkeys.h +4
-1-__BEGIN_DECLS + +#ifdef __cplusplus +extern "C" { +#endif
/* Return the number of ibpkeys */
extern int sepol_ibpkey_count(sepol_handle_t *handle,libsepol/include/sepol/ibpkeys.h +0
-0@@ -40,5 +42,9 @@ extern int sepol_ibpkey_iterate(sepol_handle_t *handle,int (*fn)(const
sepol_ibpkey_t*ibpkey,
void*fn_arg), void *arg);libsepol/include/sepol/ibpkeys.h +5
-1-__END_DECLS + +#ifdef __cplusplus +} +#endif +#endif
PATCH 2/3 libselinux: fix error when FORTIFY_SOURCE is already set
In reply to Patrick SteinhardtTwo makefiles of ours pass -D_FORTIFY_SOURCE directly to the preprocessor. While this does not pose any problems when the value has not been previously set, it can break the build if it is part of the standard build flags.
Fix the issue by undefining the flag with -Wp,-U_FORTIFY_SOURCE right before redefining the value. This fixes builds with some hardened compiler chains.
--- libselinux/src/Makefile | 3 ++- libselinux/utils/Makefile | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)
libselinux/src/Makefile +2 -1
@@ -59,7 +59,8 @@ ifeq ($(COMPILER), gcc) EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand \ -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \ -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init \ - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE=2 + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \ + -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 else EXTRA_CFLAGS = -Wunused-command-line-argument endif
libselinux/utils/Makefile +2 -1
@@ -32,7 +32,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \ -Woverflow -Wpointer-to-int-cast -Wpragmas \ -Wno-missing-field-initializers -Wno-sign-compare \ - -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \ + -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) \ + -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 \ -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \ -Werror -Wno-aggregate-return -Wno-redundant-decls \
Signature · 1 line
2.13.1
Re: [PATCH 2/3] libselinux: fix error when FORTIFY_SOURCE is already set
Quoted 10 lines · Patrick Steinhardt
Two makefiles of ours pass
-D_FORTIFY_SOURCEdirectly to the preprocessor. While this does not pose any problems when the value has
not been previously set, it can break the build if it is part of the standard build flags.Fix the issue by undefining the flag with
-Wp,-U_FORTIFY_SOURCEright
before redefining the value. This fixes builds with some hardened compiler chains.
I don't quite understand why the caller can't just specify their own CFLAGS, in which case we won't use the ones in libselinux/src/Makefile (except for the override CFLAGS, which don't include the FORTIFY options), ala:
# We turn up security all the way to 11!
make CFLAGS="-O -Wp,-D_FORTIFY_SOURCE=11"
The problem I see with your solution is that it means that if your hardened toolchain wants to select a higher _FORTIFY_SOURCE value in the future (if such a thing were to exist), we'll just undefine it and redefine to the lower value in our Makefiles. If you can't just set CFLAGS in the caller, then perhaps the libselinux Makefile should only add FORTIFY options if they aren't already defined.
Quoted 45 lines
Signed-off-by Patrick Steinhardt <[email protected]>---
libselinux/src/Makefile | 3 ++-
libselinux/utils/Makefile | 3 ++-2 files changed, 4 insertions(+), 2 deletions(-)libselinux/src/Makefile +0
-0@@ -59,7 +59,8 @@ ifeq ($(COMPILER), gcc)EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield- compat -Wsync-nand \
-Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \
-Wsuggest-attribute=noreturn -Wsuggest-attribute=purelibselinux/src/Makefile +2
-3-Wtrampolines -Wjump-misses-init \ - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE=2 + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \ + -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2else
EXTRA_CFLAGS = -Wunused-command-line-argument
endiflibselinux/utils/Makefile +0
-1@@ -32,7 +32,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi-Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
-Woverflow -Wpointer-to-int-cast -Wpragmas \
-Wno-missing-field-initializers -Wno-sign-compare \
- -Wno-format-nonliteral -Wframe-larger- than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \
+ -Wno-format-nonliteral -Wframe-larger- than=$(MAX_STACK_SIZE) \
+ -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 \
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
-Werror -Wno-aggregate-return -Wno-redundant-decls \
Re: [PATCH 2/3] libselinux: fix error when FORTIFY_SOURCE is already set
Quoted 19 lines · Stephen Smalley
Quoted 10 lines · Patrick Steinhardt
Two makefiles of ours pass
-D_FORTIFY_SOURCEdirectly to the preprocessor. While this does not pose any problems when the value has
not been previously set, it can break the build if it is part of the standard build flags.Fix the issue by undefining the flag with
-Wp,-U_FORTIFY_SOURCEright
before redefining the value. This fixes builds with some hardened compiler chains.I don't quite understand why the caller can't just specify their own CFLAGS, in which case we won't use the ones in libselinux/src/Makefile (except for the override CFLAGS, which don't include the FORTIFY options), ala:
# We turn up security all the way to 11!
make CFLAGS="-O -Wp,-D_FORTIFY_SOURCE=11"
Quoted 6 lines
The problem I see with your solution is that it means that if your hardened toolchain wants to select a higher
_FORTIFY_SOURCEvalue in the future (if such a thing were to exist), we'll just undefine it and redefine to the lower value in our Makefiles. If you can't just set CFLAGS in the caller, then perhaps the libselinuxMakefileshould only add FORTIFY options if they aren't already defined.
He said this is on gentoo in which case portage will pretty much always have CFLAGS set so this is never a problem.
Its only a problem for me if i build manually outside of portage in. I normally just sed'd out the fortify bit. But i'd agree with the best way would be to test it, something like cc-option from the kernel CFLAGS ?= $(call cc-option -Werror -Wl,-D_FORTIFY_SOURCE=2)
-- Jason
Quoted 46 lines
Quoted 46 lines
Signed-off-by Patrick Steinhardt <[email protected]>---
libselinux/src/Makefile | 3 ++-
libselinux/utils/Makefile | 3 ++-2 files changed, 4 insertions(+), 2 deletions(-)libselinux/src/Makefile +0
-0@@ -59,7 +59,8 @@ ifeq ($(COMPILER), gcc)EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield- compat -Wsync-nand \
-Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \
-Wsuggest-attribute=noreturn -Wsuggest-attribute=purelibselinux/src/Makefile +2
-3-Wtrampolines -Wjump-misses-init \ - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE=2 + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \ + -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2else
EXTRA_CFLAGS = -Wunused-command-line-argument
endiflibselinux/utils/Makefile +0
-1@@ -32,7 +32,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi-Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
-Woverflow -Wpointer-to-int-cast -Wpragmas \
-Wno-missing-field-initializers -Wno-sign-compare \
- -Wno-format-nonliteral -Wframe-larger- than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \
+ -Wno-format-nonliteral -Wframe-larger- than=$(MAX_STACK_SIZE) \
+ -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 \
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
-Werror -Wno-aggregate-return -Wno-redundant-decls \
Re: [PATCH 2/3] libselinux: fix error when FORTIFY_SOURCE is already set
In reply to Stephen SmalleyQuoted 19 lines · Stephen Smalley
Quoted 10 lines · Patrick Steinhardt
Two makefiles of ours pass
-D_FORTIFY_SOURCEdirectly to the preprocessor. While this does not pose any problems when the value has
not been previously set, it can break the build if it is part of the standard build flags.Fix the issue by undefining the flag with
-Wp,-U_FORTIFY_SOURCEright
before redefining the value. This fixes builds with some hardened compiler chains.I don't quite understand why the caller can't just specify their own CFLAGS, in which case we won't use the ones in libselinux/src/Makefile (except for the override CFLAGS, which don't include the FORTIFY options), ala:
# We turn up security all the way to 11!
make CFLAGS="-O -Wp,-D_FORTIFY_SOURCE=11"
Quoted 6 lines
The problem I see with your solution is that it means that if your hardened toolchain wants to select a higher
_FORTIFY_SOURCEvalue in the future (if such a thing were to exist), we'll just undefine it and redefine to the lower value in our Makefiles. If you can't just set CFLAGS in the caller, then perhaps the libselinuxMakefileshould only add FORTIFY options if they aren't already defined.
This does sound more reasonable than my approach, agreed. Will change as proposed in version 2, thanks.
Patrick
Quoted 46 lines
Quoted 46 lines
Signed-off-by Patrick Steinhardt <[email protected]>---
libselinux/src/Makefile | 3 ++-
libselinux/utils/Makefile | 3 ++-2 files changed, 4 insertions(+), 2 deletions(-)libselinux/src/Makefile +0
-0@@ -59,7 +59,8 @@ ifeq ($(COMPILER), gcc)EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield- compat -Wsync-nand \
-Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \
-Wsuggest-attribute=noreturn -Wsuggest-attribute=purelibselinux/src/Makefile +2
-3-Wtrampolines -Wjump-misses-init \ - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE=2 + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \ + -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2else
EXTRA_CFLAGS = -Wunused-command-line-argument
endiflibselinux/utils/Makefile +0
-1@@ -32,7 +32,8 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi-Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
-Woverflow -Wpointer-to-int-cast -Wpragmas \
-Wno-missing-field-initializers -Wno-sign-compare \
- -Wno-format-nonliteral -Wframe-larger- than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \
+ -Wno-format-nonliteral -Wframe-larger- than=$(MAX_STACK_SIZE) \
+ -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 \
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
-Werror -Wno-aggregate-return -Wno-redundant-decls \
PATCH 3/3 genhomedircon: avoid use of non-standard `getpwent_r`
In reply to Patrick SteinhardtThe 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.
--- libsemanage/src/genhomedircon.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)
libsemanage/src/genhomedircon.c +5 -17
@@ -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 ) {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 ) {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,
@@ -1131,7 +1120,6 @@ static int get_group_users(genhomedircon_settings_t * s, retval =STATUS_SUCCESS ; cleanup:endpwent (); -free (pwbuf);free (grbuf);return retval;
Signature · 1 line
2.13.1
Re: [PATCH 3/3] genhomedircon: avoid use of non-standard `getpwent_r`
Quoted 10 lines · Patrick Steinhardt
The
getpwent_rfunction is a non-standard but re-entrant version of the sdardardizedgetpwentfunction. 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 usegetpwentinstead 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.
Quoted 24 lines
Signed-off-by Patrick Steinhardt <[email protected]>---
libsemanage/src/genhomedircon.c | 22 +++++-----------------1 file changed, 5 insertions(+), 17 deletions(-)libsemanage/src/genhomedircon.c +1
-2@@ -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) {
libsemanage/src/genhomedircon.c static semanage_list_t · line 373
Quoted 7 lines
libsemanage/src/genhomedircon.c +1
-1@@ -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.
libsemanage/src/genhomedircon.c static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) · line 415
Quoted 11 lines
if (
pwbuf->pw_uid< minuid ||pwbuf->pw_uid> maxuid)
continue;
if (!semanage_list_find(shells,pwbuf->pw_shell))libsemanage/src/genhomedircon.c +0
-0@@ -413,7 +412,7 @@ static semanage_list_t *get_home_dirs (genhomedircon_settings_t * s) path = NULL; }libsemanage/src/genhomedircon.c +1
-1-if (retval && retval !=ENOENT ) { +if (errno && errno !=ENOENT ) {
Does getpwent() set errno to ENOENT? Not mentioned in its man page, unlike for getpwent_r().
Quoted 32 lines
WARN(s->h_semanage, "Error while fetching users. " "Returning list so far.");
}libsemanage/src/genhomedircon.c +1
-4@@ -1064,10 +1063,7 @@ static intget_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)libsemanage/src/genhomedircon.c +0
-0@@ -1104,15 +1100,8 @@ static int get_group_users(genhomedircon_settings_t * s, } }libsemanage/src/genhomedircon.c +0
-7- prbuflen =sysconf (_SC_GETPW_R_SIZE_MAX); -if (prbuflen <=0 ) - goto cleanup; - pwbuf =malloc (prbuflen); -if (pwbuf ==NULL ) - goto cleanup; -
setpwent();libsemanage/src/genhomedircon.c +0
-1- while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen,&pw)) == 0) {
libsemanage/src/genhomedircon.c +1
-0+while ((pw =getpwent ()) !=NULL ) {
Quoted 3 lines
// 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.
Quoted 9 lines
libsemanage/src/genhomedircon.c +0
-0@@ -1131,7 +1120,6 @@ static intget_group_users (genhomedircon_settings_t * s, retval = STATUS_SUCCESS;cleanup:
endpwent();libsemanage/src/genhomedircon.c +0
-1-free (pwbuf);free(grbuf);
return retval;
Re: [PATCH 3/3] genhomedircon: avoid use of non-standard `getpwent_r`
Quoted 12 lines · Stephen Smalley
Quoted 10 lines · Patrick Steinhardt
The
getpwent_rfunction is a non-standard but re-entrant version of the sdardardizedgetpwentfunction. 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 usegetpwentinstead to fix building with other libc implementations.
Quoted 4 lines
Not sure we are guaranteed that, since libsemanage has external users too, e.g. sssd among others. OTOH,
getpwent_rman page says that it is not really reentrant either due to shared reading position in the stream, so maybe this doesn't matter.
Hum, okay. man-pages differ here in their wording across systems, but I guess the part you're referring to is
``` The functions getpwent_r() and fgetpwent_r() are the reentrant versions of getpwent(3) and fgetpwent(3). The former reads the next passwd entry from the stream initialized by setpwent(3). The latter reads the next passwd entry from the stream fp. ```
While it indeed seems to imply that the stream by setpwent is used, they also pretend to be reentrant here. Puzzled.
Quoted 26 lines
Quoted 26 lines
Signed-off-by Patrick Steinhardt <[email protected]>---
libsemanage/src/genhomedircon.c | 22 +++++-----------------1 file changed, 5 insertions(+), 17 deletions(-)libsemanage/src/genhomedircon.c +1
-2@@ -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) {libsemanage/src/genhomedircon.c +0
-0@@ -369,7 +368,7 @@ static semanage_list_t *get_home_dirs (genhomedircon_settings_t * s) if (rbuf == NULL) goto fail;
Quoted 8 lines
libsemanage/src/genhomedircon.c static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) · line 371
Quoted 4 lines
libsemanage/src/genhomedircon.c +1
-1setpwent(); -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.
That was an oversight, yeah. Fixed in v2.
Quoted 6 lines
Quoted 6 lines
if (
pwbuf->pw_uid< minuid ||pwbuf->pw_uid> maxuid)
continue;
if (!semanage_list_find(shells,pwbuf->pw_shell))libsemanage/src/genhomedircon.c +0
-0@@ -413,7 +412,7 @@ static semanage_list_t *get_home_dirs (genhomedircon_settings_t * s)
Quoted 8 lines
libsemanage/src/genhomedircon.c static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) · line 415
Quoted 5 lines
libsemanage/src/genhomedircon.c +0
-0path = NULL; }libsemanage/src/genhomedircon.c +1
-1-if (retval && retval !=ENOENT ) { +if (errno && errno !=ENOENT ) {Does
getpwent()set errno to ENOENT? Not mentioned in its man page, unlike forgetpwent_r().
You're right, it does not. Will remove this check.
Quoted 31 lines
Quoted 31 lines
WARN(s->h_semanage, "Error while fetching users. " "Returning list so far.");
}libsemanage/src/genhomedircon.c +1
-4@@ -1064,10 +1063,7 @@ static intget_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)libsemanage/src/genhomedircon.c +0
-0@@ -1104,15 +1100,8 @@ static int get_group_users(genhomedircon_settings_t * s, } }libsemanage/src/genhomedircon.c +0
-7- prbuflen =sysconf (_SC_GETPW_R_SIZE_MAX); -if (prbuflen <=0 ) - goto cleanup; - pwbuf =malloc (prbuflen); -if (pwbuf ==NULL ) - goto cleanup; -
setpwent();libsemanage/src/genhomedircon.c +0
-1- while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen,&pw)) == 0) {
Quoted 8 lines
Quoted 4 lines
libsemanage/src/genhomedircon.c +1
-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 callsgetpwnam_r(). So if that one was ever switched back to justgetpwnam(), we'd have a potential problem there.
Agreed. But in fact, getpwnam_r is part of POSIX, so there is no need to do so here.
Quoted 9 lines
Quoted 9 lines
libsemanage/src/genhomedircon.c +0
-0@@ -1131,7 +1120,6 @@ static intget_group_users (genhomedircon_settings_t * s, retval = STATUS_SUCCESS;cleanup:
endpwent();libsemanage/src/genhomedircon.c +0
-1-free (pwbuf);free(grbuf);
return retval;
v2 cover Portability improvements
In reply to Patrick SteinhardtHi,
this is the second version of my portability fixes. Changes include applying proposed changes (thanks Stephen and Jason) as well as improved commit messages. The interdiff is attached below.
I've dropped the first patch as it's already been applied.
Patrick
Patrick Steinhardt (2):
libselinux: avoid redefining _FORTIFY_SOURCE
genhomedircon: avoid use of non-standard getpwent_r
libselinux/src/Makefile | 2 +- libselinux/utils/Makefile | 2 +- libsemanage/src/genhomedircon.c | 34 +++++++--------------------------- 3 files changed, 9 insertions(+), 29 deletions(-)
Signature · 1 line
2.13.1
libselinux/src/Makefile +1 -2
@@ -59,8 +59,7 @@ ifeq ($(COMPILER), gcc) EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand \ -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \ -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init \ - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const \ - -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE else EXTRA_CFLAGS = -Wunused-command-line-argument endif
libselinux/utils/Makefile +1 -2
@@ -32,8 +32,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \ -Woverflow -Wpointer-to-int-cast -Wpragmas \ -Wno-missing-field-initializers -Wno-sign-compare \ - -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) \ - -Wp,-U_FORTIFY_SOURCE -Wp,-D_FORTIFY_SOURCE=2 \ + -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \ -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \ -Werror -Wno-aggregate-return -Wno-redundant-decls \
libsemanage/src/genhomedircon.c +3 -11
@@ -290,9 +290,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)semanage_list_t *homedir_list =NULL ;semanage_list_t *shells =NULL ;fc_match_handle_t hand; -char *rbuf =NULL ;char *path =NULL ; -long rbuflen;uid_t temp, minuid =500 , maxuid =60000 ;int minuid_set =0 ;struct passwd *pwbuf;
@@ -361,12 +359,7 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)free (path); path =NULL ; - rbuflen =sysconf (_SC_GETPW_R_SIZE_MAX); -if (rbuflen <=0 ) - goto fail; - rbuf =malloc (rbuflen); -if (rbuf ==NULL ) - goto fail; + errno =0 ;setpwent ();while ((pwbuf =getpwent ()) !=NULL ) {if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)
@@ -410,9 +403,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) }free (path); path =NULL ; + errno =0 ; } -if (errno && errno !=ENOENT ) { +if (errno) {WARN (s->h_semanage,"Error while fetching users. " "Returning list so far." ); }
@@ -421,14 +415,12 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) goto fail;endpwent (); -free (rbuf);semanage_list_destroy (&shells);return homedir_list; fail:endpwent (); -free (rbuf);free (path);semanage_list_destroy (&homedir_list);semanage_list_destroy (&shells);
PATCH v2 1/2 libselinux: avoid redefining _FORTIFY_SOURCE
Two makefiles of ours pass -D_FORTIFY_SOURCE=2 directly to the preprocessor. While this does not pose any problems when the value has not been previously set, it can break the build if it is part of the standard build flags.
The issue can easily be fixed by instead defining _FORTIFY_SOURCE without specifying a concrete value. In this case, gcc will not error out and simply keep using the previously defined value. On the other hand, if no value has been defined, we will now compile with _FORTIFY_SOURCE=1. From feature_test_macros(7):
If _FORTIFY_SOURCE is set to 1, with compiler optimization level 1 (gcc -O1) and above, checks that shouldn't change the behavior of conforming programs are performed. With _FORTIFY_SOURCE set to 2, some more checking is added, but some conforming programs might fail.
While this leaves us with less checks for buffer overflows, it will only enable checks that should not change behaviour of conforming programs. With _FORTIFY_SOURCE=2, the compiler may even unintentionally change behaviour of conforming programs. So in fact, one could even argue that we should only be setting the value to 1 anyway to avoid surprising side effects.
So this patch changes our CFLAGS to only pass -D_FORTIFY_SOURCE without any concrete value, fixing the build issue.
--- libselinux/src/Makefile | 2 +- libselinux/utils/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
libselinux/src/Makefile +1 -1
@@ -59,7 +59,7 @@ ifeq ($(COMPILER), gcc) EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand \ -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \ -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init \ - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE=2 + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE else EXTRA_CFLAGS = -Wunused-command-line-argument endif
libselinux/utils/Makefile +1 -1
@@ -32,7 +32,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \ -Woverflow -Wpointer-to-int-cast -Wpragmas \ -Wno-missing-field-initializers -Wno-sign-compare \ - -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE=2 \ + -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \ -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \ -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \ -Werror -Wno-aggregate-return -Wno-redundant-decls \
Signature · 1 line
2.13.1
PATCH v2 2/2 genhomedircon: avoid use of non-standard `getpwent_r`
In reply to Patrick SteinhardtThe getpwent_r function is a non-standard but reentrant version of the POSIX-defined getpwent function. While it should provide the benefit of being safe to use in multi-threaded environments, it disallows us from compiling with libc implementations which stick to the POSIX standard more closely.
As libsemanage may be used in a multi-threaded environment, being reentrant may in fact be quite important to us. As such, simply switching out getpwent_r against its non-reentrant function can prove quite dangerous. But interestingly enough, the glibc implementation of getpwent_r does not even guarantee being reentrant. Quoting from getpwent_r(7):
NOTES
The function getpwent_r() is not really reentrant since it shares the reading position in the stream with all other threads.
As such, it is non-reentrant in the same sense as its simple getpwent brother and can simply be switched out without losing any guarantees here.
--- libsemanage/src/genhomedircon.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-)
libsemanage/src/genhomedircon.c +7 -27
@@ -290,14 +290,11 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)semanage_list_t *homedir_list =NULL ;semanage_list_t *shells =NULL ;fc_match_handle_t hand; -char *rbuf =NULL ;char *path =NULL ; -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) {
@@ -362,14 +359,9 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s)free (path); path =NULL ; - rbuflen =sysconf (_SC_GETPW_R_SIZE_MAX); -if (rbuflen <=0 ) - goto fail; - rbuf =malloc (rbuflen); -if (rbuf ==NULL ) - goto fail; + errno =0 ;setpwent (); -while ((retval =getpwent_r (&pwstorage, rbuf, rbuflen, &pwbuf)) ==0 ) { +while ((pwbuf =getpwent ()) !=NULL ) {if (pwbuf->pw_uid < minuid || pwbuf->pw_uid > maxuid)continue ;if (!semanage_list_find (shells, pwbuf->pw_shell))
@@ -411,9 +403,10 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) }free (path); path =NULL ; + errno =0 ; } -if (retval && retval !=ENOENT ) { +if (errno) {WARN (s->h_semanage,"Error while fetching users. " "Returning list so far." ); }
@@ -422,14 +415,12 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) goto fail;endpwent (); -free (rbuf);semanage_list_destroy (&shells);return homedir_list; fail:endpwent (); -free (rbuf);free (path);semanage_list_destroy (&homedir_list);semanage_list_destroy (&shells);
@@ -1064,10 +1055,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 +1092,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,
@@ -1131,7 +1112,6 @@ static int get_group_users(genhomedircon_settings_t * s, retval =STATUS_SUCCESS ; cleanup:endpwent (); -free (pwbuf);free (grbuf);return retval;
Signature · 1 line
2.13.1
Re: [PATCH v2 2/2] genhomedircon: avoid use of non-standard `getpwent_r`
Quoted 19 lines · Patrick Steinhardt
The
getpwent_rfunction is a non-standard but reentrant version of the
POSIX-definedgetpwentfunction. While it should provide the benefit
of being safe to use in multi-threaded environments, it disallows us from compiling with libc implementations which stick to the POSIX standard more closely.As libsemanage may be used in a multi-threaded environment, being reentrant may in fact be quite important to us. As such, simply switching out
getpwent_ragainst its non-reentrant function can prove
quite dangerous. But interestingly enough, the glibc implementation ofgetpwent_rdoes not even guarantee being reentrant. Quoting fromgetpwent_r(7):NOTES
Quoted 7 lines
The function
getpwent_r()is not really reentrant since it shares the reading position in the stream with all other threads.As such, it is non-reentrant in the same sense as its simple
getpwent
brother and can simply be switched out without losing any guarantees here.
Thanks, both patches applied.
Quoted 119 lines
Signed-off-by Patrick Steinhardt <[email protected]>---
libsemanage/src/genhomedircon.c | 34 +++++++----------------------
-----1 file changed, 7 insertions(+), 27 deletions(-)libsemanage/src/genhomedircon.c +1
-4@@ -290,14 +290,11 @@ static semanage_list_t *get_home_dirs (genhomedircon_settings_t * s) semanage_list_t *homedir_list = NULL; semanage_list_t *shells = NULL; fc_match_handle_t hand; -char *rbuf =NULL ; char *path = NULL; -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) {libsemanage/src/genhomedircon.c +0
-0@@ -362,14 +359,9 @@ static semanage_list_t *get_home_dirs (genhomedircon_settings_t * s) free(path); path = NULL;libsemanage/src/genhomedircon.c +1
-6- rbuflen =sysconf (_SC_GETPW_R_SIZE_MAX); -if (rbuflen <=0 ) - goto fail; - rbuf =malloc (rbuflen); -if (rbuf ==NULL ) - goto fail; + errno =0 ;
setpwent();libsemanage/src/genhomedircon.c +0
-1- while ((retval = getpwent_r(&pwstorage, rbuf, rbuflen,&pwbuf)) == 0) {
libsemanage/src/genhomedircon.c +1
-0+while ((pwbuf =getpwent ()) !=NULL ) {if (
pwbuf->pw_uid< minuid ||pwbuf->pw_uid> maxuid)
continue;
if (!semanage_list_find(shells,pwbuf->pw_shell))libsemanage/src/genhomedircon.c +1
-0@@ -411,9 +403,10 @@ static semanage_list_t *get_home_dirs (genhomedircon_settings_t * s) } free(path); path = NULL; + errno =0 ; }libsemanage/src/genhomedircon.c +1
-1-if (retval && retval !=ENOENT ) { +if (errno) {
WARN(s->h_semanage, "Error while fetching users. " "Returning list so far.");
}libsemanage/src/genhomedircon.c +0
-0@@ -422,14 +415,12 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) goto fail;
endpwent();libsemanage/src/genhomedircon.c +0
-1-free (rbuf);
semanage_list_destroy(&shells);
returnhomedir_list;
fail:
endpwent();libsemanage/src/genhomedircon.c +0
-1-free (rbuf);free(path);
semanage_list_destroy(&homedir_list);
semanage_list_destroy(&shells);libsemanage/src/genhomedircon.c +1
-4@@ -1064,10 +1055,7 @@ static intget_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)libsemanage/src/genhomedircon.c +0
-0@@ -1104,15 +1092,8 @@ static int get_group_users(genhomedircon_settings_t * s, } }libsemanage/src/genhomedircon.c +0
-7- prbuflen =sysconf (_SC_GETPW_R_SIZE_MAX); -if (prbuflen <=0 ) - goto cleanup; - pwbuf =malloc (prbuflen); -if (pwbuf ==NULL ) - goto cleanup; -
setpwent();libsemanage/src/genhomedircon.c +0
-1- while ((retval = getpwent_r(&pwstorage, pwbuf, prbuflen,&pw)) == 0) {
libsemanage/src/genhomedircon.c +1
-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,libsemanage/src/genhomedircon.c +0
-0@@ -1131,7 +1112,6 @@ static intget_group_users (genhomedircon_settings_t * s, retval = STATUS_SUCCESS;cleanup:
endpwent();libsemanage/src/genhomedircon.c +0
-1-free (pwbuf);free(grbuf);
return retval;