From cbb38e0b21f5aa72204f6d9ebc15e18b63ac3211 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 01:14:10 +0100 Subject: [PATCH 01/12] src/usermod.c: wsfix Fix white space. Signed-off-by: Alejandro Colomar --- src/usermod.c | 215 ++++++++++++++++++++------------------------------ 1 file changed, 87 insertions(+), 128 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index ba01186227..87e905eb4c 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -664,19 +664,19 @@ static void new_spent (struct spwd *spent, bool process_selinux) */ NORETURN static void -fail_exit (int code, bool process_selinux) +fail_exit(int code, bool process_selinux) { #ifdef ENABLE_SUBIDS if (sub_gid_locked) { - if (sub_gid_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ()); + if (sub_gid_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); /* continue */ } } if (sub_uid_locked) { - if (sub_uid_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ()); + if (sub_uid_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); /* continue */ } @@ -684,30 +684,30 @@ fail_exit (int code, bool process_selinux) #endif /* ENABLE_SUBIDS */ #ifdef SHADOWGRP if (sgr_locked) { - if (sgr_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname ()); + if (sgr_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); /* continue */ } } #endif if (gr_locked) { - if (gr_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname ()); + if (gr_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); /* continue */ } } if (spw_locked) { - if (spw_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname ()); + if (spw_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); /* continue */ } } if (pw_locked) { - if (pw_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname ()); + if (pw_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); /* continue */ } @@ -1514,26 +1514,26 @@ static void close_files(const struct option_flags *flags) #ifdef ENABLE_SUBIDS if (sub_gid_locked) { - if (sub_gid_close (process_selinux) == 0) { - fprintf (stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname ()); + if (sub_gid_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sub_gid_dbname()); - fail_exit (E_SUB_GID_UPDATE, process_selinux); + fail_exit(E_SUB_GID_UPDATE, process_selinux); } - if (sub_gid_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname ()); + if (sub_gid_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); /* continue */ } sub_gid_locked = false; } if (sub_uid_locked) { - if (sub_uid_close (process_selinux) == 0) { - fprintf (stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname ()); + if (sub_uid_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sub_uid_dbname()); - fail_exit (E_SUB_UID_UPDATE, process_selinux); + fail_exit(E_SUB_UID_UPDATE, process_selinux); } - if (sub_uid_unlock (process_selinux) == 0) { - fprintf (stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname ()); + if (sub_uid_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); /* continue */ } @@ -1544,69 +1544,51 @@ static void close_files(const struct option_flags *flags) if (gr_locked) { #ifdef SHADOWGRP if (is_shadow_grp) { - if (sgr_close (process_selinux) == 0) { - fprintf (stderr, - _("%s: failure while writing changes to %s\n"), - Prog, sgr_dbname ()); - SYSLOG(LOG_ERR, "failure while writing changes to %s", - sgr_dbname()); - fail_exit (E_GRP_UPDATE, process_selinux); + if (sgr_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sgr_dbname()); + SYSLOG(LOG_ERR, "failure while writing changes to %s", sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } - if (sgr_unlock (process_selinux) == 0) { - fprintf (stderr, - _("%s: failed to unlock %s\n"), - Prog, sgr_dbname ()); + if (sgr_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); /* continue */ } sgr_locked = false; } #endif - if (gr_close (process_selinux) == 0) { - fprintf (stderr, - _("%s: failure while writing changes to %s\n"), - Prog, gr_dbname ()); - SYSLOG(LOG_ERR, "failure while writing changes to %s", - gr_dbname()); - fail_exit (E_GRP_UPDATE, process_selinux); + if (gr_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, gr_dbname()); + SYSLOG(LOG_ERR, "failure while writing changes to %s", gr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } - if (gr_unlock (process_selinux) == 0) { - fprintf (stderr, - _("%s: failed to unlock %s\n"), - Prog, gr_dbname ()); + if (gr_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); /* continue */ } gr_locked = false; } if (spw_locked) { - if (spw_close (process_selinux) == 0) { - fprintf (stderr, - _("%s: failure while writing changes to %s\n"), - Prog, spw_dbname ()); + if (spw_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", spw_dbname()); - fail_exit (E_PW_UPDATE, process_selinux); + fail_exit(E_PW_UPDATE, process_selinux); } - if (spw_unlock (process_selinux) == 0) { - fprintf (stderr, - _("%s: failed to unlock %s\n"), - Prog, spw_dbname ()); + if (spw_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); /* continue */ } spw_locked = false; } - if (pw_close (process_selinux) == 0) { - fprintf (stderr, - _("%s: failure while writing changes to %s\n"), - Prog, pw_dbname ()); + if (pw_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", pw_dbname()); - fail_exit (E_PW_UPDATE, process_selinux); + fail_exit(E_PW_UPDATE, process_selinux); } - if (pw_unlock (process_selinux) == 0) { - fprintf (stderr, - _("%s: failed to unlock %s\n"), - Prog, pw_dbname ()); + if (pw_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); /* continue */ } @@ -1616,11 +1598,11 @@ static void close_files(const struct option_flags *flags) * Close the DBM and/or flat files */ #ifdef SHADOWGRP - endsgent (); + endsgent(); #endif - endgrent (); - endspent (); - endpwent (); + endgrent(); + endspent(); + endpwent(); } /* @@ -1628,34 +1610,27 @@ static void close_files(const struct option_flags *flags) * * open_files() opens the two password files. */ -static void open_files (bool process_selinux) +static void +open_files(bool process_selinux) { - if (pw_lock () == 0) { - fprintf (stderr, - _("%s: cannot lock %s; try again later.\n"), - Prog, pw_dbname ()); - fail_exit (E_PW_UPDATE, process_selinux); + if (pw_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, pw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); } pw_locked = true; - if (pw_open (O_CREAT | O_RDWR) == 0) { - fprintf (stderr, - _("%s: cannot open %s\n"), - Prog, pw_dbname ()); - fail_exit (E_PW_UPDATE, process_selinux); + if (pw_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, pw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); } if (is_shadow_pwd && (lflg || pflg || eflg || fflg || Lflg || Uflg)) { - if (spw_lock () == 0) { - fprintf (stderr, - _("%s: cannot lock %s; try again later.\n"), - Prog, spw_dbname ()); - fail_exit (E_PW_UPDATE, process_selinux); + if (spw_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, spw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); } spw_locked = true; - if (is_shadow_pwd && (spw_open (O_CREAT | O_RDWR) == 0)) { - fprintf (stderr, - _("%s: cannot open %s\n"), - Prog, spw_dbname ()); - fail_exit (E_PW_UPDATE, process_selinux); + if (is_shadow_pwd && (spw_open(O_CREAT | O_RDWR) == 0)) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, spw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); } } @@ -1664,64 +1639,48 @@ static void open_files (bool process_selinux) * Lock and open the group file. This will load all of the * group entries. */ - if (gr_lock () == 0) { - fprintf (stderr, - _("%s: cannot lock %s; try again later.\n"), - Prog, gr_dbname ()); - fail_exit (E_GRP_UPDATE, process_selinux); + if (gr_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, gr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } gr_locked = true; - if (gr_open (O_CREAT | O_RDWR) == 0) { - fprintf (stderr, - _("%s: cannot open %s\n"), - Prog, gr_dbname ()); - fail_exit (E_GRP_UPDATE, process_selinux); + if (gr_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, gr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } #ifdef SHADOWGRP - if (is_shadow_grp && (sgr_lock () == 0)) { - fprintf (stderr, - _("%s: cannot lock %s; try again later.\n"), - Prog, sgr_dbname ()); - fail_exit (E_GRP_UPDATE, process_selinux); + if (is_shadow_grp && (sgr_lock() == 0)) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } sgr_locked = true; - if (is_shadow_grp && (sgr_open (O_CREAT | O_RDWR) == 0)) { - fprintf (stderr, - _("%s: cannot open %s\n"), - Prog, sgr_dbname ()); - fail_exit (E_GRP_UPDATE, process_selinux); + if (is_shadow_grp && (sgr_open(O_CREAT | O_RDWR) == 0)) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } #endif } #ifdef ENABLE_SUBIDS if (vflg || Vflg) { - if (sub_uid_lock () == 0) { - fprintf (stderr, - _("%s: cannot lock %s; try again later.\n"), - Prog, sub_uid_dbname ()); - fail_exit (E_SUB_UID_UPDATE, process_selinux); + if (sub_uid_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sub_uid_dbname()); + fail_exit(E_SUB_UID_UPDATE, process_selinux); } sub_uid_locked = true; - if (sub_uid_open (O_CREAT | O_RDWR) == 0) { - fprintf (stderr, - _("%s: cannot open %s\n"), - Prog, sub_uid_dbname ()); - fail_exit (E_SUB_UID_UPDATE, process_selinux); + if (sub_uid_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, sub_uid_dbname()); + fail_exit(E_SUB_UID_UPDATE, process_selinux); } } if (wflg || Wflg) { - if (sub_gid_lock () == 0) { - fprintf (stderr, - _("%s: cannot lock %s; try again later.\n"), - Prog, sub_gid_dbname ()); - fail_exit (E_SUB_GID_UPDATE, process_selinux); + if (sub_gid_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sub_gid_dbname()); + fail_exit(E_SUB_GID_UPDATE, process_selinux); } sub_gid_locked = true; - if (sub_gid_open (O_CREAT | O_RDWR) == 0) { - fprintf (stderr, - _("%s: cannot open %s\n"), - Prog, sub_gid_dbname ()); - fail_exit (E_SUB_GID_UPDATE, process_selinux); + if (sub_gid_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, sub_gid_dbname()); + fail_exit(E_SUB_GID_UPDATE, process_selinux); } } #endif /* ENABLE_SUBIDS */ From 571a4429c999a2ba8631b51c4523ddce5e5032ab Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 01:16:56 +0100 Subject: [PATCH 02/12] src/usermod.c: Remove dead code If we're here, 'is_shadow_pwd' is true. See the enclosing conditional. Fixes: 1e0f5299 (2026-03-17; "usermod: only lock shadow file as needed") Cc: Pat Riehecky Signed-off-by: Alejandro Colomar --- src/usermod.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/usermod.c b/src/usermod.c index 87e905eb4c..6fc4b90c8e 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1628,7 +1628,7 @@ open_files(bool process_selinux) fail_exit(E_PW_UPDATE, process_selinux); } spw_locked = true; - if (is_shadow_pwd && (spw_open(O_CREAT | O_RDWR) == 0)) { + if (spw_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, spw_dbname()); fail_exit(E_PW_UPDATE, process_selinux); } From 885d6025618e78a31cdd17021aa675ebf6135c10 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 01:20:40 +0100 Subject: [PATCH 03/12] src/usermod.c: Set 'sgr_locked = true' only if we really locked Fixes: 24e742d2 (2007-11-17; "* src/usermod.c (fail_exit): Add static variables pw_locked,...") Cc: Pat Riehecky Signed-off-by: Alejandro Colomar --- src/usermod.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 6fc4b90c8e..6bb0e31105 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1649,14 +1649,16 @@ open_files(bool process_selinux) fail_exit(E_GRP_UPDATE, process_selinux); } #ifdef SHADOWGRP - if (is_shadow_grp && (sgr_lock() == 0)) { - fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); - } - sgr_locked = true; - if (is_shadow_grp && (sgr_open(O_CREAT | O_RDWR) == 0)) { - fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + if (is_shadow_grp) { + if (sgr_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); + } + sgr_locked = true; + if (sgr_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); + } } #endif } From bf195bdbd7e80ec51a4d1966ad5fadd0b27925ae Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 02:32:15 +0100 Subject: [PATCH 04/12] src/usermod.c: Only close files if they were locked Fixes: e65380f5 (2026-03-17; "usermod: only unlock on close_files if we actually locked them") Cc: Pat Riehecky Signed-off-by: Alejandro Colomar --- src/usermod.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 6bb0e31105..8424549525 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1543,7 +1543,7 @@ static void close_files(const struct option_flags *flags) if (gr_locked) { #ifdef SHADOWGRP - if (is_shadow_grp) { + if (sgr_locked) { if (sgr_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sgr_dbname()); @@ -1582,17 +1582,19 @@ static void close_files(const struct option_flags *flags) } spw_locked = false; } - if (pw_close(process_selinux) == 0) { - fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, pw_dbname()); - SYSLOG(LOG_ERR, "failure while writing changes to %s", pw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); - } - if (pw_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); - /* continue */ + if (pw_locked) { + if (pw_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, pw_dbname()); + SYSLOG(LOG_ERR, "failure while writing changes to %s", pw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); + } + if (pw_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); + SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); + /* continue */ + } + pw_locked = false; } - pw_locked = false; /* * Close the DBM and/or flat files From 1afafd75d3f6ef1cc8ca4a3a8be0cad39c8359ee Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 02:53:44 +0100 Subject: [PATCH 05/12] src/usermod.c: Remove unnecessary nesting We only care about sgr_locked for closing&unlocking gshadow. Fixes: e65380f5 (2026-03-17; "usermod: only unlock on close_files if we actually locked them") Signed-off-by: Alejandro Colomar --- src/usermod.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 8424549525..be87a1a3b2 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1541,22 +1541,22 @@ static void close_files(const struct option_flags *flags) } #endif /* ENABLE_SUBIDS */ - if (gr_locked) { #ifdef SHADOWGRP - if (sgr_locked) { - if (sgr_close(process_selinux) == 0) { - fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sgr_dbname()); - SYSLOG(LOG_ERR, "failure while writing changes to %s", sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); - } - if (sgr_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); - /* continue */ - } - sgr_locked = false; + if (sgr_locked) { + if (sgr_close(process_selinux) == 0) { + fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sgr_dbname()); + SYSLOG(LOG_ERR, "failure while writing changes to %s", sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } + if (sgr_unlock(process_selinux) == 0) { + fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); + SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); + /* continue */ + } + sgr_locked = false; + } #endif + if (gr_locked) { if (gr_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", gr_dbname()); From adcc49eea0217d6f4efd2d1991a3a001cd65aff6 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 02:44:13 +0100 Subject: [PATCH 06/12] src/usermod.c: Merge variables into structure Signed-off-by: Alejandro Colomar --- src/usermod.c | 73 +++++++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index be87a1a3b2..b4cba16748 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -111,6 +111,20 @@ struct option_flags { bool prefix; }; +struct lk_db_files { + bool pw; + bool spw; + bool gr; +#ifdef SHADOWGRP + bool sgr; +#endif +#ifdef ENABLE_SUBIDS + bool subuid; + bool subgid; +#endif +}; + + /* * Global variables */ @@ -183,16 +197,7 @@ static bool is_sub_uid = false; static bool is_sub_gid = false; #endif /* ENABLE_SUBIDS */ -static bool pw_locked = false; -static bool spw_locked = false; -static bool gr_locked = false; -#ifdef SHADOWGRP -static bool sgr_locked = false; -#endif -#ifdef ENABLE_SUBIDS -static bool sub_uid_locked = false; -static bool sub_gid_locked = false; -#endif /* ENABLE_SUBIDS */ +static struct lk_db_files lk = {}; /* local function prototypes */ @@ -667,14 +672,14 @@ static void fail_exit(int code, bool process_selinux) { #ifdef ENABLE_SUBIDS - if (sub_gid_locked) { + if (lk.subgid) { if (sub_gid_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); /* continue */ } } - if (sub_uid_locked) { + if (lk.subuid) { if (sub_uid_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); @@ -683,7 +688,7 @@ fail_exit(int code, bool process_selinux) } #endif /* ENABLE_SUBIDS */ #ifdef SHADOWGRP - if (sgr_locked) { + if (lk.sgr) { if (sgr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); @@ -691,21 +696,21 @@ fail_exit(int code, bool process_selinux) } } #endif - if (gr_locked) { + if (lk.gr) { if (gr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); /* continue */ } } - if (spw_locked) { + if (lk.spw) { if (spw_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); /* continue */ } } - if (pw_locked) { + if (lk.pw) { if (pw_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); @@ -1513,7 +1518,7 @@ static void close_files(const struct option_flags *flags) process_selinux = !flags->chroot && !flags->prefix; #ifdef ENABLE_SUBIDS - if (sub_gid_locked) { + if (lk.subgid) { if (sub_gid_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sub_gid_dbname()); @@ -1524,9 +1529,9 @@ static void close_files(const struct option_flags *flags) SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); /* continue */ } - sub_gid_locked = false; + lk.subgid = false; } - if (sub_uid_locked) { + if (lk.subuid) { if (sub_uid_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sub_uid_dbname()); @@ -1537,12 +1542,12 @@ static void close_files(const struct option_flags *flags) SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); /* continue */ } - sub_uid_locked = false; + lk.subuid = false; } #endif /* ENABLE_SUBIDS */ #ifdef SHADOWGRP - if (sgr_locked) { + if (lk.sgr) { if (sgr_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sgr_dbname()); @@ -1553,10 +1558,10 @@ static void close_files(const struct option_flags *flags) SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); /* continue */ } - sgr_locked = false; + lk.sgr = false; } #endif - if (gr_locked) { + if (lk.gr) { if (gr_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", gr_dbname()); @@ -1567,9 +1572,9 @@ static void close_files(const struct option_flags *flags) SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); /* continue */ } - gr_locked = false; + lk.gr = false; } - if (spw_locked) { + if (lk.spw) { if (spw_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", spw_dbname()); @@ -1580,9 +1585,9 @@ static void close_files(const struct option_flags *flags) SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); /* continue */ } - spw_locked = false; + lk.spw = false; } - if (pw_locked) { + if (lk.pw) { if (pw_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", pw_dbname()); @@ -1593,7 +1598,7 @@ static void close_files(const struct option_flags *flags) SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); /* continue */ } - pw_locked = false; + lk.pw = false; } /* @@ -1619,7 +1624,7 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, pw_dbname()); fail_exit(E_PW_UPDATE, process_selinux); } - pw_locked = true; + lk.pw = true; if (pw_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, pw_dbname()); fail_exit(E_PW_UPDATE, process_selinux); @@ -1629,7 +1634,7 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, spw_dbname()); fail_exit(E_PW_UPDATE, process_selinux); } - spw_locked = true; + lk.spw = true; if (spw_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, spw_dbname()); fail_exit(E_PW_UPDATE, process_selinux); @@ -1645,7 +1650,7 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, gr_dbname()); fail_exit(E_GRP_UPDATE, process_selinux); } - gr_locked = true; + lk.gr = true; if (gr_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, gr_dbname()); fail_exit(E_GRP_UPDATE, process_selinux); @@ -1656,7 +1661,7 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); fail_exit(E_GRP_UPDATE, process_selinux); } - sgr_locked = true; + lk.sgr = true; if (sgr_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); fail_exit(E_GRP_UPDATE, process_selinux); @@ -1670,7 +1675,7 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sub_uid_dbname()); fail_exit(E_SUB_UID_UPDATE, process_selinux); } - sub_uid_locked = true; + lk.subuid = true; if (sub_uid_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, sub_uid_dbname()); fail_exit(E_SUB_UID_UPDATE, process_selinux); @@ -1681,7 +1686,7 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sub_gid_dbname()); fail_exit(E_SUB_GID_UPDATE, process_selinux); } - sub_gid_locked = true; + lk.subgid = true; if (sub_gid_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, sub_gid_dbname()); fail_exit(E_SUB_GID_UPDATE, process_selinux); From b2a024d55cdf7c078db46e67db39413b7ff98c63 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 02:59:54 +0100 Subject: [PATCH 07/12] src/usermod.c: fail_exit(): Unset lk fields after unlocking This is unnecessary here, but it will help splitting this code into a library function. Signed-off-by: Alejandro Colomar --- src/usermod.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index b4cba16748..87ba34781f 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -676,14 +676,16 @@ fail_exit(int code, bool process_selinux) if (sub_gid_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); - /* continue */ + } else { + lk.subgid = false; } } if (lk.subuid) { if (sub_uid_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); - /* continue */ + } else { + lk.subuid = false; } } #endif /* ENABLE_SUBIDS */ @@ -692,7 +694,8 @@ fail_exit(int code, bool process_selinux) if (sgr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); - /* continue */ + } else { + lk.sgr = false; } } #endif @@ -700,21 +703,24 @@ fail_exit(int code, bool process_selinux) if (gr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); - /* continue */ + } else { + lk.gr = false; } } if (lk.spw) { if (spw_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); - /* continue */ + } else { + lk.spw = false; } } if (lk.pw) { if (pw_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); - /* continue */ + } else { + lk.pw = false; } } From 8b932f19fea1ccece396eef51c6b374368a205c1 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 03:02:05 +0100 Subject: [PATCH 08/12] src/usermod.c: Split unlock_db() helper from fail_exit() Signed-off-by: Alejandro Colomar --- src/usermod.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 87ba34781f..ab8416276b 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -664,12 +664,10 @@ static void new_spent (struct spwd *spent, bool process_selinux) } } -/* - * fail_exit - exit with an error code after unlocking files - */ -NORETURN + +// unlock_db - unlock database files static void -fail_exit(int code, bool process_selinux) +unlock_db(bool process_selinux) { #ifdef ENABLE_SUBIDS if (lk.subgid) { @@ -688,8 +686,8 @@ fail_exit(int code, bool process_selinux) lk.subuid = false; } } -#endif /* ENABLE_SUBIDS */ -#ifdef SHADOWGRP +#endif +#ifdef SHADOWGRP if (lk.sgr) { if (sgr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); @@ -723,6 +721,17 @@ fail_exit(int code, bool process_selinux) lk.pw = false; } } +} + + +/* + * fail_exit - exit with an error code after unlocking files + */ +NORETURN +static void +fail_exit(int code, bool process_selinux) +{ + unlock_db(process_selinux); #ifdef WITH_AUDIT audit_logger (AUDIT_USER_MGMT, From f4753f50b1a17a0c61fd7ca8b319c44dabdccd51 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 03:19:32 +0100 Subject: [PATCH 09/12] src/usermod.c: Split close_db() helper from close_files() It closes and unlocks, but doesn't fail_exit(). Instead, it reports an error code. Signed-off-by: Alejandro Colomar --- src/usermod.c | 69 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index ab8416276b..e9739044dd 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1520,29 +1520,24 @@ process_flags(int argc, char **argv, struct option_flags *flags) #endif /* ENABLE_SUBIDS */ } -/* - * close_files - close all of the files that were opened - * - * close_files() closes all of the files that were opened for this new - * user. This causes any modified entries to be written out. - */ -static void close_files(const struct option_flags *flags) -{ - bool process_selinux; - process_selinux = !flags->chroot && !flags->prefix; +// close_db - close and unlock database files +static int +close_db(bool process_selinux) +{ + int ret; #ifdef ENABLE_SUBIDS if (lk.subgid) { if (sub_gid_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sub_gid_dbname()); - fail_exit(E_SUB_GID_UPDATE, process_selinux); + ret = E_SUB_GID_UPDATE; + goto fail; } if (sub_gid_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); - /* continue */ } lk.subgid = false; } @@ -1550,28 +1545,27 @@ static void close_files(const struct option_flags *flags) if (sub_uid_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sub_uid_dbname()); - fail_exit(E_SUB_UID_UPDATE, process_selinux); + ret = E_SUB_UID_UPDATE; + goto fail; } if (sub_uid_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); - /* continue */ } lk.subuid = false; } -#endif /* ENABLE_SUBIDS */ - +#endif #ifdef SHADOWGRP if (lk.sgr) { if (sgr_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + ret = E_GRP_UPDATE; + goto fail; } if (sgr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); - /* continue */ } lk.sgr = false; } @@ -1580,12 +1574,12 @@ static void close_files(const struct option_flags *flags) if (gr_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", gr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + ret = E_GRP_UPDATE; + goto fail; } if (gr_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); - /* continue */ } lk.gr = false; } @@ -1593,12 +1587,12 @@ static void close_files(const struct option_flags *flags) if (spw_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", spw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + ret = E_PW_UPDATE; + goto fail; } if (spw_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); - /* continue */ } lk.spw = false; } @@ -1606,25 +1600,44 @@ static void close_files(const struct option_flags *flags) if (pw_close(process_selinux) == 0) { fprintf(stderr, _("%s: failure while writing changes to %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failure while writing changes to %s", pw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + ret = E_PW_UPDATE; + goto fail; } if (pw_unlock(process_selinux) == 0) { fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); - /* continue */ } lk.pw = false; } - /* - * Close the DBM and/or flat files - */ -#ifdef SHADOWGRP +#ifdef SHADOWGRP endsgent(); #endif endgrent(); endspent(); endpwent(); + return 0; +fail: + unlock_db(process_selinux); + return ret; +} + +/* + * close_files - close all of the files that were opened + * + * close_files() closes all of the files that were opened for this new + * user. This causes any modified entries to be written out. + */ +static void close_files(const struct option_flags *flags) +{ + int ret; + bool process_selinux; + + process_selinux = !flags->chroot && !flags->prefix; + + ret = close_db(process_selinux); + if (ret != 0) + fail_exit(ret, process_selinux); } /* From 830dcf201bcf486f516383d67798876f8ac30971 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 03:23:39 +0100 Subject: [PATCH 10/12] src/usermod.c: close_db(): First close all files, then unlock all files This is simpler, as we can re-use unlock_db(). Signed-off-by: Alejandro Colomar --- src/usermod.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index e9739044dd..19e0f73dfe 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1527,6 +1527,7 @@ close_db(bool process_selinux) { int ret; + ret = 0; #ifdef ENABLE_SUBIDS if (lk.subgid) { if (sub_gid_close(process_selinux) == 0) { @@ -1535,11 +1536,6 @@ close_db(bool process_selinux) ret = E_SUB_GID_UPDATE; goto fail; } - if (sub_gid_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_gid_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", sub_gid_dbname()); - } - lk.subgid = false; } if (lk.subuid) { if (sub_uid_close(process_selinux) == 0) { @@ -1548,11 +1544,6 @@ close_db(bool process_selinux) ret = E_SUB_UID_UPDATE; goto fail; } - if (sub_uid_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sub_uid_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", sub_uid_dbname()); - } - lk.subuid = false; } #endif #ifdef SHADOWGRP @@ -1563,11 +1554,6 @@ close_db(bool process_selinux) ret = E_GRP_UPDATE; goto fail; } - if (sgr_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, sgr_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", sgr_dbname()); - } - lk.sgr = false; } #endif if (lk.gr) { @@ -1577,11 +1563,6 @@ close_db(bool process_selinux) ret = E_GRP_UPDATE; goto fail; } - if (gr_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, gr_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", gr_dbname()); - } - lk.gr = false; } if (lk.spw) { if (spw_close(process_selinux) == 0) { @@ -1590,11 +1571,6 @@ close_db(bool process_selinux) ret = E_PW_UPDATE; goto fail; } - if (spw_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, spw_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", spw_dbname()); - } - lk.spw = false; } if (lk.pw) { if (pw_close(process_selinux) == 0) { @@ -1603,11 +1579,6 @@ close_db(bool process_selinux) ret = E_PW_UPDATE; goto fail; } - if (pw_unlock(process_selinux) == 0) { - fprintf(stderr, _("%s: failed to unlock %s\n"), Prog, pw_dbname()); - SYSLOG(LOG_ERR, "failed to unlock %s", pw_dbname()); - } - lk.pw = false; } #ifdef SHADOWGRP @@ -1616,7 +1587,6 @@ close_db(bool process_selinux) endgrent(); endspent(); endpwent(); - return 0; fail: unlock_db(process_selinux); return ret; From fae19e8f58b192345b9f63e07969bd76572a6a17 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 03:35:15 +0100 Subject: [PATCH 11/12] src/usermod.c: open_files(): Refactor conditionals This prepares for the following commit. Signed-off-by: Alejandro Colomar --- src/usermod.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 19e0f73dfe..16cd5004bf 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1618,14 +1618,16 @@ static void close_files(const struct option_flags *flags) static void open_files(bool process_selinux) { - if (pw_lock() == 0) { - fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, pw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); - } - lk.pw = true; - if (pw_open(O_CREAT | O_RDWR) == 0) { - fprintf(stderr, _("%s: cannot open %s\n"), Prog, pw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + if (true) { + if (pw_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, pw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); + } + lk.pw = true; + if (pw_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, pw_dbname()); + fail_exit(E_PW_UPDATE, process_selinux); + } } if (is_shadow_pwd && (lflg || pflg || eflg || fflg || Lflg || Uflg)) { if (spw_lock() == 0) { @@ -1638,7 +1640,6 @@ open_files(bool process_selinux) fail_exit(E_PW_UPDATE, process_selinux); } } - if (Gflg || lflg) { /* * Lock and open the group file. This will load all of the @@ -1653,20 +1654,20 @@ open_files(bool process_selinux) fprintf(stderr, _("%s: cannot open %s\n"), Prog, gr_dbname()); fail_exit(E_GRP_UPDATE, process_selinux); } + } #ifdef SHADOWGRP - if (is_shadow_grp) { - if (sgr_lock() == 0) { - fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); - } - lk.sgr = true; - if (sgr_open(O_CREAT | O_RDWR) == 0) { - fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); - } + if ((Gflg || lflg) && is_shadow_grp) { + if (sgr_lock() == 0) { + fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); + } + lk.sgr = true; + if (sgr_open(O_CREAT | O_RDWR) == 0) { + fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); + fail_exit(E_GRP_UPDATE, process_selinux); } -#endif } +#endif #ifdef ENABLE_SUBIDS if (vflg || Vflg) { if (sub_uid_lock() == 0) { @@ -1690,7 +1691,7 @@ open_files(bool process_selinux) fail_exit(E_SUB_GID_UPDATE, process_selinux); } } -#endif /* ENABLE_SUBIDS */ +#endif } /* From a0eeda3163eeee14f59d10f36fe83afe527464c9 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Tue, 17 Mar 2026 02:38:09 +0100 Subject: [PATCH 12/12] src/usermod.c: Split open_db() from open_files() Signed-off-by: Alejandro Colomar --- src/usermod.c | 100 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 29 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 16cd5004bf..fd8f1ee741 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -1610,88 +1610,130 @@ static void close_files(const struct option_flags *flags) fail_exit(ret, process_selinux); } -/* - * open_files - lock and open the password files - * - * open_files() opens the two password files. - */ -static void -open_files(bool process_selinux) +// open_db - lock and open database files +static int +open_db(struct lk_db_files *f, bool process_selinux) { - if (true) { + int ret; + + if (f->spw) + f->pw = true; + if (f->pw) { if (pw_lock() == 0) { fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, pw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + ret = E_PW_UPDATE; + goto fail; } lk.pw = true; if (pw_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, pw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + ret = E_PW_UPDATE; + goto fail; } } - if (is_shadow_pwd && (lflg || pflg || eflg || fflg || Lflg || Uflg)) { + if (f->spw) { if (spw_lock() == 0) { fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, spw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + ret = E_PW_UPDATE; + goto fail; } lk.spw = true; if (spw_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, spw_dbname()); - fail_exit(E_PW_UPDATE, process_selinux); + ret = E_PW_UPDATE; + goto fail; } } - if (Gflg || lflg) { - /* - * Lock and open the group file. This will load all of the - * group entries. - */ + +#ifdef SHADOWGRP + if (f->sgr) + f->gr = true; +#endif + if (f->gr) { if (gr_lock() == 0) { fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, gr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + ret = E_GRP_UPDATE; + goto fail; } lk.gr = true; if (gr_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, gr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + ret = E_GRP_UPDATE; + goto fail; } } #ifdef SHADOWGRP - if ((Gflg || lflg) && is_shadow_grp) { + if (f->sgr) { if (sgr_lock() == 0) { fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + ret = E_GRP_UPDATE; + goto fail; } lk.sgr = true; if (sgr_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, sgr_dbname()); - fail_exit(E_GRP_UPDATE, process_selinux); + ret = E_GRP_UPDATE; + goto fail; } } #endif + #ifdef ENABLE_SUBIDS - if (vflg || Vflg) { + if (f->subuid) { if (sub_uid_lock() == 0) { fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sub_uid_dbname()); - fail_exit(E_SUB_UID_UPDATE, process_selinux); + ret = E_SUB_UID_UPDATE; + goto fail; } lk.subuid = true; if (sub_uid_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, sub_uid_dbname()); - fail_exit(E_SUB_UID_UPDATE, process_selinux); + ret = E_SUB_UID_UPDATE; + goto fail; } } - if (wflg || Wflg) { + if (f->subgid) { if (sub_gid_lock() == 0) { fprintf(stderr, _("%s: cannot lock %s; try again later.\n"), Prog, sub_gid_dbname()); - fail_exit(E_SUB_GID_UPDATE, process_selinux); + ret = E_SUB_GID_UPDATE; + goto fail; } lk.subgid = true; if (sub_gid_open(O_CREAT | O_RDWR) == 0) { fprintf(stderr, _("%s: cannot open %s\n"), Prog, sub_gid_dbname()); - fail_exit(E_SUB_GID_UPDATE, process_selinux); + ret = E_SUB_GID_UPDATE; + goto fail; } } #endif + return 0; +fail: + unlock_db(process_selinux); + return ret; +} + + +static void +open_files(bool process_selinux) +{ + int ret; + + struct lk_db_files f = { + .pw = true, + .spw = is_shadow_pwd && (lflg || pflg || eflg || fflg || Lflg || Uflg), + .gr = Gflg || lflg, + #ifdef SHADOWGRP + .sgr = (Gflg || lflg) && is_shadow_grp, + #endif + #ifdef ENABLE_SUBIDS + .subuid = vflg || Vflg, + .subgid = wflg || Wflg, + #endif + }; + + ret = open_db(&f, process_selinux); + if (ret != 0) + fail_exit(ret, process_selinux); } /*