* [PATCH] genhomedircon: cleanup parsing of uid config values @ 2025-10-21 11:36 Rahul Sandhu 2025-10-21 13:15 ` Stephen Smalley 0 siblings, 1 reply; 10+ messages in thread From: Rahul Sandhu @ 2025-10-21 11:36 UTC (permalink / raw) To: selinux; +Cc: Rahul Sandhu Parsing KV files with a separator of similar format is fairly similar, so we may as well add a helper function to make it easier to read. Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> --- libsemanage/src/genhomedircon.c | 81 +++++++++++++-------------------- 1 file changed, 32 insertions(+), 49 deletions(-) diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c index 34056562..08a22df1 100644 --- a/libsemanage/src/genhomedircon.c +++ b/libsemanage/src/genhomedircon.c @@ -308,6 +308,29 @@ done: return retval; } +static int parse_uid_config(const char *file, const char *key, const char *sep, + uid_t fallback, uid_t *out) +{ + char *path = semanage_findval(file, key, sep); + if (!path || !*path) { + free(path); + *out = fallback; + return 0; + } + + char *endptr; + const unsigned long val = strtoul(path, &endptr, 0); + free(path); + + if (endptr != path && *endptr == '\0') { + *out = (uid_t)val; + return 0; + } + + *out = fallback; + return -1; +} + static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) { semanage_list_t *homedir_list = NULL; @@ -315,7 +338,6 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) fc_match_handle_t hand; char *path = NULL; uid_t temp, minuid = 500, maxuid = 60000; - int minuid_set = 0; struct passwd *pwbuf; struct stat buf; @@ -362,56 +384,17 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) "Conversion failed for key " key ", is its value a number?" \ " Falling back to default value of `%s`.", #val); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - minuid = (uid_t)val; - minuid_set = 1; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - minuid = FALLBACK_MINUID; - minuid_set = 1; - } - } - free(path); - path = NULL; + if (parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid) < 0) + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - maxuid = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); - maxuid = FALLBACK_MAXUID; - } - } - free(path); - path = NULL; + if (parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid) < 0) + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - temp = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); - temp = FALLBACK_LU_UIDNUMBER; - } - if (!minuid_set || temp < minuid) { - minuid = temp; - minuid_set = 1; - } - } - free(path); - path = NULL; + if (parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp) < 0) + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); + + if (temp < minuid) + minuid = temp; #undef genhomedircon_warn_conv_fail -- 2.51.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] genhomedircon: cleanup parsing of uid config values 2025-10-21 11:36 [PATCH] genhomedircon: cleanup parsing of uid config values Rahul Sandhu @ 2025-10-21 13:15 ` Stephen Smalley 2025-10-22 0:01 ` [PATCH v2] " Rahul Sandhu 0 siblings, 1 reply; 10+ messages in thread From: Stephen Smalley @ 2025-10-21 13:15 UTC (permalink / raw) To: Rahul Sandhu; +Cc: selinux On Tue, Oct 21, 2025 at 7:36 AM Rahul Sandhu <nvraxn@gmail.com> wrote: > > Parsing KV files with a separator of similar format is fairly similar, > so we may as well add a helper function to make it easier to read. > > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> > --- > libsemanage/src/genhomedircon.c | 81 +++++++++++++-------------------- > 1 file changed, 32 insertions(+), 49 deletions(-) > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index 34056562..08a22df1 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -308,6 +308,29 @@ done: > return retval; > } > > +static int parse_uid_config(const char *file, const char *key, const char *sep, > + uid_t fallback, uid_t *out) > +{ > + char *path = semanage_findval(file, key, sep); Not new to this patch, but more obvious now that it is in a dedicated helper: seems odd to call this path when it is a UID string. > + if (!path || !*path) { > + free(path); > + *out = fallback; > + return 0; > + } > + > + char *endptr; > + const unsigned long val = strtoul(path, &endptr, 0); > + free(path); > + > + if (endptr != path && *endptr == '\0') { This could end up dereferencing the memory you freed above. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] genhomedircon: cleanup parsing of uid config values 2025-10-21 13:15 ` Stephen Smalley @ 2025-10-22 0:01 ` Rahul Sandhu 2025-10-22 14:51 ` Stephen Smalley 0 siblings, 1 reply; 10+ messages in thread From: Rahul Sandhu @ 2025-10-22 0:01 UTC (permalink / raw) To: stephen.smalley.work; +Cc: nvraxn, selinux Parsing KV files with a separator of similar format is fairly similar, so we may as well add a helper function to make it easier to read. Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> --- libsemanage/src/genhomedircon.c | 82 +++++++++++++-------------------- 1 file changed, 33 insertions(+), 49 deletions(-) v2: rename path to something more sensible (afterall, we are parsing a UID!) and move the free to later, just before both return paths to not dereference it when checking whether we actually parsed a valid number or not. diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c index 34056562..bb840856 100644 --- a/libsemanage/src/genhomedircon.c +++ b/libsemanage/src/genhomedircon.c @@ -308,6 +308,30 @@ done: return retval; } +static int parse_uid_config(const char *file, const char *key, const char *sep, + uid_t fallback, uid_t *out) +{ + char *uid_str = semanage_findval(file, key, sep); + if (!uid_str || !*uid_str) { + free(uid_str); + *out = fallback; + return 0; + } + + char *endptr; + const unsigned long val = strtoul(uid_str, &endptr, 0); + + if (endptr != uid_str && *endptr == '\0') { + *out = (uid_t)val; + free(uid_str); + return 0; + } + + free(uid_str); + *out = fallback; + return -1; +} + static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) { semanage_list_t *homedir_list = NULL; @@ -315,7 +339,6 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) fc_match_handle_t hand; char *path = NULL; uid_t temp, minuid = 500, maxuid = 60000; - int minuid_set = 0; struct passwd *pwbuf; struct stat buf; @@ -362,56 +385,17 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) "Conversion failed for key " key ", is its value a number?" \ " Falling back to default value of `%s`.", #val); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - minuid = (uid_t)val; - minuid_set = 1; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - minuid = FALLBACK_MINUID; - minuid_set = 1; - } - } - free(path); - path = NULL; + if (parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid) < 0) + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - maxuid = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); - maxuid = FALLBACK_MAXUID; - } - } - free(path); - path = NULL; + if (parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid) < 0) + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - temp = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); - temp = FALLBACK_LU_UIDNUMBER; - } - if (!minuid_set || temp < minuid) { - minuid = temp; - minuid_set = 1; - } - } - free(path); - path = NULL; + if (parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp) < 0) + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); + + if (temp < minuid) + minuid = temp; #undef genhomedircon_warn_conv_fail -- 2.51.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] genhomedircon: cleanup parsing of uid config values 2025-10-22 0:01 ` [PATCH v2] " Rahul Sandhu @ 2025-10-22 14:51 ` Stephen Smalley 2025-10-24 8:07 ` [PATCH v3] " Rahul Sandhu 0 siblings, 1 reply; 10+ messages in thread From: Stephen Smalley @ 2025-10-22 14:51 UTC (permalink / raw) To: Rahul Sandhu; +Cc: selinux On Tue, Oct 21, 2025 at 8:01 PM Rahul Sandhu <nvraxn@gmail.com> wrote: > > Parsing KV files with a separator of similar format is fairly similar, > so we may as well add a helper function to make it easier to read. > > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> > --- > libsemanage/src/genhomedircon.c | 82 +++++++++++++-------------------- > 1 file changed, 33 insertions(+), 49 deletions(-) > > v2: rename path to something more sensible (afterall, we are parsing a > UID!) and move the free to later, just before both return paths to > not dereference it when checking whether we actually parsed a valid > number or not. > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index 34056562..bb840856 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -315,7 +339,6 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) > fc_match_handle_t hand; > char *path = NULL; > uid_t temp, minuid = 500, maxuid = 60000; Should we be using our FALLBACK_* definitions above to initialize? Or alternatively, do we need to initialize them at all since parse_uid_config() always sets them? > - int minuid_set = 0; Not sure you can drop this altogether, see below. > @@ -362,56 +385,17 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) > "Conversion failed for key " key ", is its value a number?" \ > " Falling back to default value of `%s`.", #val); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - minuid = (uid_t)val; > - minuid_set = 1; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > - minuid = FALLBACK_MINUID; > - minuid_set = 1; > - } > - } Note that minuid_set is NOT set to 1 if !path or !*path i.e. no UID_MIN definition found at all in login.defs. > - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - temp = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > - temp = FALLBACK_LU_UIDNUMBER; > - } > - if (!minuid_set || temp < minuid) { > - minuid = temp; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + if (parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp) < 0) > + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > + > + if (temp < minuid) > + minuid = temp; This might lower minuid to FALLBACK_LU_UIDNUMBER (500) even if it was explicitly set by login.defs. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] genhomedircon: cleanup parsing of uid config values 2025-10-22 14:51 ` Stephen Smalley @ 2025-10-24 8:07 ` Rahul Sandhu 2025-10-24 19:29 ` Stephen Smalley 0 siblings, 1 reply; 10+ messages in thread From: Rahul Sandhu @ 2025-10-24 8:07 UTC (permalink / raw) To: stephen.smalley.work; +Cc: nvraxn, selinux Parsing KV files with a separator of similar format is fairly similar, so we may as well add a helper function to make it easier to read. Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> --- libsemanage/src/genhomedircon.c | 107 +++++++++++++++++--------------- 1 file changed, 57 insertions(+), 50 deletions(-) v2: rename path to something more sensible (afterall, we are parsing a UID!) and move the free to later, just before both return paths to not dereference it when checking whether we actually parsed a valid number or not. v3: handle the fallback case for minuid properly such that we don't end up always using a fallback if minuid is not set in login.defs, and return a bool instead as it's a bit more sensible for what we're trying to return. Also, check for ERANGE. diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c index 34056562..e91c64e6 100644 --- a/libsemanage/src/genhomedircon.c +++ b/libsemanage/src/genhomedircon.c @@ -308,14 +308,52 @@ done: return retval; } +/* + * Parses `file` for `key` seperated by `sep` into `out`. + * Returns: + * true on success. + * false on failure. + * `out` is guaranteed to be initalised. + * `fallback_set` is initalised to false, and set to true if a fallback was used. + */ +static bool parse_uid_config(const char *file, const char *key, const char *sep, + uid_t fallback, uid_t *out, bool *fallback_set) +{ + assert(out); + assert(fallback_set); + + *fallback_set = false; + + char *uid_str = semanage_findval(file, key, sep); + if (!uid_str || !*uid_str) { + free(uid_str); + *fallback_set = true; + *out = fallback; + return true; + } + + char *endptr; + errno = 0; + const unsigned long val = strtoul(uid_str, &endptr, 0); + + if (endptr != uid_str && *endptr == '\0' && errno != ERANGE) { + *out = (uid_t)val; + free(uid_str); + return true; + } + + free(uid_str); + *fallback_set = true; + *out = fallback; + return false; +} + 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 *path = NULL; - uid_t temp, minuid = 500, maxuid = 60000; - int minuid_set = 0; struct passwd *pwbuf; struct stat buf; @@ -362,56 +400,25 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) "Conversion failed for key " key ", is its value a number?" \ " Falling back to default value of `%s`.", #val); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - minuid = (uid_t)val; - minuid_set = 1; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - minuid = FALLBACK_MINUID; - minuid_set = 1; - } - } - free(path); - path = NULL; + uid_t minuid; + bool fallback_set; + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid, &fallback_set)) + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - maxuid = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); - maxuid = FALLBACK_MAXUID; - } - } - free(path); - path = NULL; + const bool logindefs_minuid_fallback_set = fallback_set; - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - temp = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); - temp = FALLBACK_LU_UIDNUMBER; - } - if (!minuid_set || temp < minuid) { - minuid = temp; - minuid_set = 1; - } - } - free(path); - path = NULL; + uid_t temp; + if (!parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp, &fallback_set)) + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); + + if (logindefs_minuid_fallback_set) + minuid = temp; + + uid_t maxuid; + /* We don't actually check fallback_set here, PATH_ETC_LOGIN_DEFS is the one source of + truth for UID_MAX. */ + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid, &fallback_set)) + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); #undef genhomedircon_warn_conv_fail -- 2.51.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] genhomedircon: cleanup parsing of uid config values 2025-10-24 8:07 ` [PATCH v3] " Rahul Sandhu @ 2025-10-24 19:29 ` Stephen Smalley 2025-10-26 19:13 ` [PATCH v4] " Rahul Sandhu 0 siblings, 1 reply; 10+ messages in thread From: Stephen Smalley @ 2025-10-24 19:29 UTC (permalink / raw) To: Rahul Sandhu; +Cc: selinux On Fri, Oct 24, 2025 at 4:08 AM Rahul Sandhu <nvraxn@gmail.com> wrote: > > Parsing KV files with a separator of similar format is fairly similar, > so we may as well add a helper function to make it easier to read. > > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > libsemanage/src/genhomedircon.c | 107 +++++++++++++++++--------------- > 1 file changed, 57 insertions(+), 50 deletions(-) > > v2: rename path to something more sensible (afterall, we are parsing a > UID!) and move the free to later, just before both return paths to > not dereference it when checking whether we actually parsed a valid > number or not. > v3: handle the fallback case for minuid properly such that we don't end > up always using a fallback if minuid is not set in login.defs, and > return a bool instead as it's a bit more sensible for what we're > trying to return. Also, check for ERANGE. > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index 34056562..e91c64e6 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -308,14 +308,52 @@ done: > return retval; > } > > +/* > + * Parses `file` for `key` seperated by `sep` into `out`. > + * Returns: > + * true on success. > + * false on failure. > + * `out` is guaranteed to be initalised. > + * `fallback_set` is initalised to false, and set to true if a fallback was used. > + */ > +static bool parse_uid_config(const char *file, const char *key, const char *sep, > + uid_t fallback, uid_t *out, bool *fallback_set) > +{ > + assert(out); > + assert(fallback_set); > + > + *fallback_set = false; > + > + char *uid_str = semanage_findval(file, key, sep); > + if (!uid_str || !*uid_str) { > + free(uid_str); > + *fallback_set = true; > + *out = fallback; > + return true; > + } > + > + char *endptr; > + errno = 0; > + const unsigned long val = strtoul(uid_str, &endptr, 0); > + > + if (endptr != uid_str && *endptr == '\0' && errno != ERANGE) { > + *out = (uid_t)val; > + free(uid_str); > + return true; > + } > + > + free(uid_str); > + *fallback_set = true; > + *out = fallback; > + return false; > +} > + > 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 *path = NULL; > - uid_t temp, minuid = 500, maxuid = 60000; > - int minuid_set = 0; > struct passwd *pwbuf; > struct stat buf; > > @@ -362,56 +400,25 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) > "Conversion failed for key " key ", is its value a number?" \ > " Falling back to default value of `%s`.", #val); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - minuid = (uid_t)val; > - minuid_set = 1; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > - minuid = FALLBACK_MINUID; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + uid_t minuid; > + bool fallback_set; > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid, &fallback_set)) > + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - maxuid = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > - maxuid = FALLBACK_MAXUID; > - } > - } > - free(path); > - path = NULL; > + const bool logindefs_minuid_fallback_set = fallback_set; > > - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - temp = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > - temp = FALLBACK_LU_UIDNUMBER; > - } > - if (!minuid_set || temp < minuid) { > - minuid = temp; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + uid_t temp; > + if (!parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp, &fallback_set)) > + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > + > + if (logindefs_minuid_fallback_set) > + minuid = temp; > + > + uid_t maxuid; > + /* We don't actually check fallback_set here, PATH_ETC_LOGIN_DEFS is the one source of > + truth for UID_MAX. */ > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid, &fallback_set)) > + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > > #undef genhomedircon_warn_conv_fail > > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4] genhomedircon: cleanup parsing of uid config values 2025-10-24 19:29 ` Stephen Smalley @ 2025-10-26 19:13 ` Rahul Sandhu 2025-10-27 13:10 ` Stephen Smalley 2025-10-27 16:54 ` Stephen Smalley 0 siblings, 2 replies; 10+ messages in thread From: Rahul Sandhu @ 2025-10-26 19:13 UTC (permalink / raw) To: stephen.smalley.work; +Cc: nvraxn, selinux, hylandb256, Hyland B. Parsing KV files with a separator of similar format is fairly similar, so we may as well add a helper function to make it easier to read. Credit to Hyland for reminding me to check for ERANGE. Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> Reviewed-by: Hyland B. <me@ow.swag.toys> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> --- libsemanage/src/genhomedircon.c | 107 +++++++++++++++++--------------- 1 file changed, 57 insertions(+), 50 deletions(-) v2: rename path to something more sensible (afterall, we are parsing a UID!) and move the free to later, just before both return paths to not dereference it when checking whether we actually parsed a valid number or not. v3: handle the fallback case for minuid properly such that we don't end up always using a fallback if minuid is not set in login.defs, and return a bool instead as it's a bit more sensible for what we're trying to return. Also, check for ERANGE. v4: add credit to Hyland for reminding me to check for ERANGE. diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c index 34056562..e91c64e6 100644 --- a/libsemanage/src/genhomedircon.c +++ b/libsemanage/src/genhomedircon.c @@ -308,14 +308,52 @@ done: return retval; } +/* + * Parses `file` for `key` seperated by `sep` into `out`. + * Returns: + * true on success. + * false on failure. + * `out` is guaranteed to be initalised. + * `fallback_set` is initalised to false, and set to true if a fallback was used. + */ +static bool parse_uid_config(const char *file, const char *key, const char *sep, + uid_t fallback, uid_t *out, bool *fallback_set) +{ + assert(out); + assert(fallback_set); + + *fallback_set = false; + + char *uid_str = semanage_findval(file, key, sep); + if (!uid_str || !*uid_str) { + free(uid_str); + *fallback_set = true; + *out = fallback; + return true; + } + + char *endptr; + errno = 0; + const unsigned long val = strtoul(uid_str, &endptr, 0); + + if (endptr != uid_str && *endptr == '\0' && errno != ERANGE) { + *out = (uid_t)val; + free(uid_str); + return true; + } + + free(uid_str); + *fallback_set = true; + *out = fallback; + return false; +} + 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 *path = NULL; - uid_t temp, minuid = 500, maxuid = 60000; - int minuid_set = 0; struct passwd *pwbuf; struct stat buf; @@ -362,56 +400,25 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) "Conversion failed for key " key ", is its value a number?" \ " Falling back to default value of `%s`.", #val); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - minuid = (uid_t)val; - minuid_set = 1; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - minuid = FALLBACK_MINUID; - minuid_set = 1; - } - } - free(path); - path = NULL; + uid_t minuid; + bool fallback_set; + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid, &fallback_set)) + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - maxuid = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); - maxuid = FALLBACK_MAXUID; - } - } - free(path); - path = NULL; + const bool logindefs_minuid_fallback_set = fallback_set; - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); - if (path && *path) { - char *endptr; - const unsigned long val = strtoul(path, &endptr, 0); - if (endptr != path && *endptr == '\0') { - temp = (uid_t)val; - } else { - /* we were provided an invalid value, use defaults. */ - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); - temp = FALLBACK_LU_UIDNUMBER; - } - if (!minuid_set || temp < minuid) { - minuid = temp; - minuid_set = 1; - } - } - free(path); - path = NULL; + uid_t temp; + if (!parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp, &fallback_set)) + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); + + if (logindefs_minuid_fallback_set) + minuid = temp; + + uid_t maxuid; + /* We don't actually check fallback_set here, PATH_ETC_LOGIN_DEFS is the one source of + truth for UID_MAX. */ + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid, &fallback_set)) + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); #undef genhomedircon_warn_conv_fail -- 2.51.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genhomedircon: cleanup parsing of uid config values 2025-10-26 19:13 ` [PATCH v4] " Rahul Sandhu @ 2025-10-27 13:10 ` Stephen Smalley 2025-10-27 13:12 ` Stephen Smalley 2025-10-27 16:54 ` Stephen Smalley 1 sibling, 1 reply; 10+ messages in thread From: Stephen Smalley @ 2025-10-27 13:10 UTC (permalink / raw) To: Rahul Sandhu; +Cc: selinux, hylandb256, Hyland B. On Sun, Oct 26, 2025 at 3:13 PM Rahul Sandhu <nvraxn@gmail.com> wrote: > > Parsing KV files with a separator of similar format is fairly similar, > so we may as well add a helper function to make it easier to read. > > Credit to Hyland for reminding me to check for ERANGE. > > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> > Reviewed-by: Hyland B. <me@ow.swag.toys> I don't see any such email from Hyland, just this from you, possibly it was bounced by the list or eaten by spam filters? > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> > --- > libsemanage/src/genhomedircon.c | 107 +++++++++++++++++--------------- > 1 file changed, 57 insertions(+), 50 deletions(-) > > v2: rename path to something more sensible (afterall, we are parsing a > UID!) and move the free to later, just before both return paths to > not dereference it when checking whether we actually parsed a valid > number or not. > v3: handle the fallback case for minuid properly such that we don't end > up always using a fallback if minuid is not set in login.defs, and > return a bool instead as it's a bit more sensible for what we're > trying to return. Also, check for ERANGE. > v4: add credit to Hyland for reminding me to check for ERANGE. > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index 34056562..e91c64e6 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -308,14 +308,52 @@ done: > return retval; > } > > +/* > + * Parses `file` for `key` seperated by `sep` into `out`. > + * Returns: > + * true on success. > + * false on failure. > + * `out` is guaranteed to be initalised. > + * `fallback_set` is initalised to false, and set to true if a fallback was used. > + */ > +static bool parse_uid_config(const char *file, const char *key, const char *sep, > + uid_t fallback, uid_t *out, bool *fallback_set) > +{ > + assert(out); > + assert(fallback_set); > + > + *fallback_set = false; > + > + char *uid_str = semanage_findval(file, key, sep); > + if (!uid_str || !*uid_str) { > + free(uid_str); > + *fallback_set = true; > + *out = fallback; > + return true; > + } > + > + char *endptr; > + errno = 0; > + const unsigned long val = strtoul(uid_str, &endptr, 0); > + > + if (endptr != uid_str && *endptr == '\0' && errno != ERANGE) { > + *out = (uid_t)val; > + free(uid_str); > + return true; > + } > + > + free(uid_str); > + *fallback_set = true; > + *out = fallback; > + return false; > +} > + > 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 *path = NULL; > - uid_t temp, minuid = 500, maxuid = 60000; > - int minuid_set = 0; > struct passwd *pwbuf; > struct stat buf; > > @@ -362,56 +400,25 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) > "Conversion failed for key " key ", is its value a number?" \ > " Falling back to default value of `%s`.", #val); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - minuid = (uid_t)val; > - minuid_set = 1; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > - minuid = FALLBACK_MINUID; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + uid_t minuid; > + bool fallback_set; > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid, &fallback_set)) > + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - maxuid = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > - maxuid = FALLBACK_MAXUID; > - } > - } > - free(path); > - path = NULL; > + const bool logindefs_minuid_fallback_set = fallback_set; > > - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - temp = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > - temp = FALLBACK_LU_UIDNUMBER; > - } > - if (!minuid_set || temp < minuid) { > - minuid = temp; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + uid_t temp; > + if (!parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp, &fallback_set)) > + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > + > + if (logindefs_minuid_fallback_set) > + minuid = temp; > + > + uid_t maxuid; > + /* We don't actually check fallback_set here, PATH_ETC_LOGIN_DEFS is the one source of > + truth for UID_MAX. */ > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid, &fallback_set)) > + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > > #undef genhomedircon_warn_conv_fail > > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genhomedircon: cleanup parsing of uid config values 2025-10-27 13:10 ` Stephen Smalley @ 2025-10-27 13:12 ` Stephen Smalley 0 siblings, 0 replies; 10+ messages in thread From: Stephen Smalley @ 2025-10-27 13:12 UTC (permalink / raw) To: Rahul Sandhu; +Cc: selinux, hylandb256 On Mon, Oct 27, 2025 at 9:10 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > On Sun, Oct 26, 2025 at 3:13 PM Rahul Sandhu <nvraxn@gmail.com> wrote: > > > > Parsing KV files with a separator of similar format is fairly similar, > > so we may as well add a helper function to make it easier to read. > > > > Credit to Hyland for reminding me to check for ERANGE. > > > > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> > > Reviewed-by: Hyland B. <me@ow.swag.toys> > > I don't see any such email from Hyland, just this from you, possibly > it was bounced by the list or eaten by spam filters? That address bounces with no such domain, so I am not taking this as a Reviewed-by tag. > > > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> > > --- > > libsemanage/src/genhomedircon.c | 107 +++++++++++++++++--------------- > > 1 file changed, 57 insertions(+), 50 deletions(-) > > > > v2: rename path to something more sensible (afterall, we are parsing a > > UID!) and move the free to later, just before both return paths to > > not dereference it when checking whether we actually parsed a valid > > number or not. > > v3: handle the fallback case for minuid properly such that we don't end > > up always using a fallback if minuid is not set in login.defs, and > > return a bool instead as it's a bit more sensible for what we're > > trying to return. Also, check for ERANGE. > > v4: add credit to Hyland for reminding me to check for ERANGE. > > > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > > index 34056562..e91c64e6 100644 > > --- a/libsemanage/src/genhomedircon.c > > +++ b/libsemanage/src/genhomedircon.c > > @@ -308,14 +308,52 @@ done: > > return retval; > > } > > > > +/* > > + * Parses `file` for `key` seperated by `sep` into `out`. > > + * Returns: > > + * true on success. > > + * false on failure. > > + * `out` is guaranteed to be initalised. > > + * `fallback_set` is initalised to false, and set to true if a fallback was used. > > + */ > > +static bool parse_uid_config(const char *file, const char *key, const char *sep, > > + uid_t fallback, uid_t *out, bool *fallback_set) > > +{ > > + assert(out); > > + assert(fallback_set); > > + > > + *fallback_set = false; > > + > > + char *uid_str = semanage_findval(file, key, sep); > > + if (!uid_str || !*uid_str) { > > + free(uid_str); > > + *fallback_set = true; > > + *out = fallback; > > + return true; > > + } > > + > > + char *endptr; > > + errno = 0; > > + const unsigned long val = strtoul(uid_str, &endptr, 0); > > + > > + if (endptr != uid_str && *endptr == '\0' && errno != ERANGE) { > > + *out = (uid_t)val; > > + free(uid_str); > > + return true; > > + } > > + > > + free(uid_str); > > + *fallback_set = true; > > + *out = fallback; > > + return false; > > +} > > + > > 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 *path = NULL; > > - uid_t temp, minuid = 500, maxuid = 60000; > > - int minuid_set = 0; > > struct passwd *pwbuf; > > struct stat buf; > > > > @@ -362,56 +400,25 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) > > "Conversion failed for key " key ", is its value a number?" \ > > " Falling back to default value of `%s`.", #val); > > > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); > > - if (path && *path) { > > - char *endptr; > > - const unsigned long val = strtoul(path, &endptr, 0); > > - if (endptr != path && *endptr == '\0') { > > - minuid = (uid_t)val; > > - minuid_set = 1; > > - } else { > > - /* we were provided an invalid value, use defaults. */ > > - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > > - minuid = FALLBACK_MINUID; > > - minuid_set = 1; > > - } > > - } > > - free(path); > > - path = NULL; > > + uid_t minuid; > > + bool fallback_set; > > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid, &fallback_set)) > > + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > > > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); > > - if (path && *path) { > > - char *endptr; > > - const unsigned long val = strtoul(path, &endptr, 0); > > - if (endptr != path && *endptr == '\0') { > > - maxuid = (uid_t)val; > > - } else { > > - /* we were provided an invalid value, use defaults. */ > > - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > > - maxuid = FALLBACK_MAXUID; > > - } > > - } > > - free(path); > > - path = NULL; > > + const bool logindefs_minuid_fallback_set = fallback_set; > > > > - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); > > - if (path && *path) { > > - char *endptr; > > - const unsigned long val = strtoul(path, &endptr, 0); > > - if (endptr != path && *endptr == '\0') { > > - temp = (uid_t)val; > > - } else { > > - /* we were provided an invalid value, use defaults. */ > > - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > > - temp = FALLBACK_LU_UIDNUMBER; > > - } > > - if (!minuid_set || temp < minuid) { > > - minuid = temp; > > - minuid_set = 1; > > - } > > - } > > - free(path); > > - path = NULL; > > + uid_t temp; > > + if (!parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp, &fallback_set)) > > + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > > + > > + if (logindefs_minuid_fallback_set) > > + minuid = temp; > > + > > + uid_t maxuid; > > + /* We don't actually check fallback_set here, PATH_ETC_LOGIN_DEFS is the one source of > > + truth for UID_MAX. */ > > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid, &fallback_set)) > > + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > > > > #undef genhomedircon_warn_conv_fail > > > > -- > > 2.51.0 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] genhomedircon: cleanup parsing of uid config values 2025-10-26 19:13 ` [PATCH v4] " Rahul Sandhu 2025-10-27 13:10 ` Stephen Smalley @ 2025-10-27 16:54 ` Stephen Smalley 1 sibling, 0 replies; 10+ messages in thread From: Stephen Smalley @ 2025-10-27 16:54 UTC (permalink / raw) To: Rahul Sandhu; +Cc: selinux, hhhyland.belcherrr4 On Sun, Oct 26, 2025 at 3:13 PM Rahul Sandhu <nvraxn@gmail.com> wrote: > > Parsing KV files with a separator of similar format is fairly similar, > so we may as well add a helper function to make it easier to read. > > Credit to Hyland for reminding me to check for ERANGE. > > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com> > Reviewed-by: Hyland Belcher <hhhyland.belcherrr4@gmail.com> > Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com> Thanks, applied with the corrected tag. > --- > libsemanage/src/genhomedircon.c | 107 +++++++++++++++++--------------- > 1 file changed, 57 insertions(+), 50 deletions(-) > > v2: rename path to something more sensible (afterall, we are parsing a > UID!) and move the free to later, just before both return paths to > not dereference it when checking whether we actually parsed a valid > number or not. > v3: handle the fallback case for minuid properly such that we don't end > up always using a fallback if minuid is not set in login.defs, and > return a bool instead as it's a bit more sensible for what we're > trying to return. Also, check for ERANGE. > v4: add credit to Hyland for reminding me to check for ERANGE. > > diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c > index 34056562..e91c64e6 100644 > --- a/libsemanage/src/genhomedircon.c > +++ b/libsemanage/src/genhomedircon.c > @@ -308,14 +308,52 @@ done: > return retval; > } > > +/* > + * Parses `file` for `key` seperated by `sep` into `out`. > + * Returns: > + * true on success. > + * false on failure. > + * `out` is guaranteed to be initalised. > + * `fallback_set` is initalised to false, and set to true if a fallback was used. > + */ > +static bool parse_uid_config(const char *file, const char *key, const char *sep, > + uid_t fallback, uid_t *out, bool *fallback_set) > +{ > + assert(out); > + assert(fallback_set); > + > + *fallback_set = false; > + > + char *uid_str = semanage_findval(file, key, sep); > + if (!uid_str || !*uid_str) { > + free(uid_str); > + *fallback_set = true; > + *out = fallback; > + return true; > + } > + > + char *endptr; > + errno = 0; > + const unsigned long val = strtoul(uid_str, &endptr, 0); > + > + if (endptr != uid_str && *endptr == '\0' && errno != ERANGE) { > + *out = (uid_t)val; > + free(uid_str); > + return true; > + } > + > + free(uid_str); > + *fallback_set = true; > + *out = fallback; > + return false; > +} > + > 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 *path = NULL; > - uid_t temp, minuid = 500, maxuid = 60000; > - int minuid_set = 0; > struct passwd *pwbuf; > struct stat buf; > > @@ -362,56 +400,25 @@ static semanage_list_t *get_home_dirs(genhomedircon_settings_t * s) > "Conversion failed for key " key ", is its value a number?" \ > " Falling back to default value of `%s`.", #val); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - minuid = (uid_t)val; > - minuid_set = 1; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > - minuid = FALLBACK_MINUID; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + uid_t minuid; > + bool fallback_set; > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MIN", NULL, FALLBACK_MINUID, &minuid, &fallback_set)) > + genhomedircon_warn_conv_fail("UID_MIN", FALLBACK_MINUID); > > - path = semanage_findval(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - maxuid = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > - maxuid = FALLBACK_MAXUID; > - } > - } > - free(path); > - path = NULL; > + const bool logindefs_minuid_fallback_set = fallback_set; > > - path = semanage_findval(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "="); > - if (path && *path) { > - char *endptr; > - const unsigned long val = strtoul(path, &endptr, 0); > - if (endptr != path && *endptr == '\0') { > - temp = (uid_t)val; > - } else { > - /* we were provided an invalid value, use defaults. */ > - genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > - temp = FALLBACK_LU_UIDNUMBER; > - } > - if (!minuid_set || temp < minuid) { > - minuid = temp; > - minuid_set = 1; > - } > - } > - free(path); > - path = NULL; > + uid_t temp; > + if (!parse_uid_config(PATH_ETC_LIBUSER, "LU_UIDNUMBER", "=", FALLBACK_LU_UIDNUMBER, &temp, &fallback_set)) > + genhomedircon_warn_conv_fail("LU_UIDNUMBER", FALLBACK_LU_UIDNUMBER); > + > + if (logindefs_minuid_fallback_set) > + minuid = temp; > + > + uid_t maxuid; > + /* We don't actually check fallback_set here, PATH_ETC_LOGIN_DEFS is the one source of > + truth for UID_MAX. */ > + if (!parse_uid_config(PATH_ETC_LOGIN_DEFS, "UID_MAX", NULL, FALLBACK_MAXUID, &maxuid, &fallback_set)) > + genhomedircon_warn_conv_fail("UID_MAX", FALLBACK_MAXUID); > > #undef genhomedircon_warn_conv_fail > > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-10-27 16:54 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-21 11:36 [PATCH] genhomedircon: cleanup parsing of uid config values Rahul Sandhu 2025-10-21 13:15 ` Stephen Smalley 2025-10-22 0:01 ` [PATCH v2] " Rahul Sandhu 2025-10-22 14:51 ` Stephen Smalley 2025-10-24 8:07 ` [PATCH v3] " Rahul Sandhu 2025-10-24 19:29 ` Stephen Smalley 2025-10-26 19:13 ` [PATCH v4] " Rahul Sandhu 2025-10-27 13:10 ` Stephen Smalley 2025-10-27 13:12 ` Stephen Smalley 2025-10-27 16:54 ` Stephen Smalley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).