]> git.ipfire.org Git - thirdparty/shadow.git/commitdiff
useradd: don't mask user_id variable in check_uid_range master
authorSerge Hallyn <serge@hallyn.com>
Tue, 21 Jul 2026 19:41:09 +0000 (14:41 -0500)
committerAlejandro Colomar <foss+github@alejandro-colomar.es>
Tue, 21 Jul 2026 21:37:04 +0000 (23:37 +0200)
While it's not "wrong" right now, it could be confusing, and cause
trouble later.

I'm not a fan of global variables in the first place, but restructuring
venerable code like this tends to be asking for trouble.

Also rename rflg as sys_user.  And fix its type, as it is a bool, not
int.

Also a note on commit 764e56e9ec: useradd: fix subuid allocation,
which is in a PR comment.  Putting it here so it'll be in the git
tree itself:

> For posterity: this was apparently always broken, but the opposite way. The
> user_id part of the check was always true if !user_id, and user_id was always
> 0, so that was ignored. Alex fixed that accidentally by switching to if
> (user_id == 0) return false, in 8508d61.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
Reported-by: Alejandro Colomar <alx@kernel.org>
Reviewed-by: Alejandro Colomar <alx@kernel.org>
src/useradd.c

index a56d737123ec67604d027b44ddfaa991921f9c1a..fd8e36b8a87871d0cff5780be72bca5c0ef8ae92 100644 (file)
@@ -254,7 +254,7 @@ static void usr_update (unsigned long subuid_count, unsigned long subgid_count,
                         const struct option_flags *flags);
 static void create_home(const struct option_flags *flags);
 static void create_mail(const struct option_flags *flags);
-static void check_uid_range(int rflg, uid_t user_id);
+static void check_uid_range(bool sys_user, uid_t uid);
 
 
 /*
@@ -2339,21 +2339,21 @@ static void create_mail(const struct option_flags *flags)
 #endif
 }
 
-static void check_uid_range(int rflg, uid_t user_id)
+static void check_uid_range(bool sys_user, uid_t uid)
 {
        uid_t uid_min ;
        uid_t uid_max ;
-       if (rflg) {
+       if (sys_user) {
                uid_max = getdef_ulong("SYS_UID_MAX",getdef_ulong("UID_MIN",1000UL)-1);
-               if (user_id > uid_max) {
-                       eprintf(_("%s warning: %s's uid %d is greater than SYS_UID_MAX %d\n"), Prog, user_name, user_id, uid_max);
+               if (uid > uid_max) {
+                       eprintf(_("%s warning: %s's uid %d is greater than SYS_UID_MAX %d\n"), Prog, user_name, uid, uid_max);
                }
        }else{
                uid_min = getdef_ulong("UID_MIN", 1000UL);
                uid_max = getdef_ulong("UID_MAX", 6000UL);
                if (uid_min <= uid_max) {
-                       if (user_id < uid_min || user_id >uid_max)
-                               eprintf(_("%s warning: %s's uid %d outside of the UID_MIN %d and UID_MAX %d range.\n"), Prog, user_name, user_id, uid_min, uid_max);
+                       if (uid < uid_min || uid > uid_max)
+                               eprintf(_("%s warning: %s's uid %d outside of the UID_MIN %d and UID_MAX %d range.\n"), Prog, user_name, uid, uid_min, uid_max);
                }
        }
 
@@ -2543,7 +2543,7 @@ int main (int argc, char **argv)
 #endif                         /* ENABLE_SUBIDS */
 
        if (uflg)
-          check_uid_range(rflg,user_id);
+          check_uid_range(rflg, user_id);
 #ifdef WITH_TCB
        if (getdef_bool ("USE_TCB")) {
                if (shadowtcb_create (user_name, user_id) == SHADOWTCB_FAILURE) {