From b20b0b66066cef87da779b92aa7a18c7c3f71a2c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 9 May 2017 09:37:37 +0200 Subject: [PATCH 1/3] sysusers: split make_files() This patch extracts the code which is in charge to write the new users or groups into temporary files and move it into 4 dedicated functions. This part was previously inlined in makes_files() making this function quite big and hard to read and maintain. There should be no functional change. --- src/sysusers/sysusers.c | 721 ++++++++++++++++++++++------------------ 1 file changed, 397 insertions(+), 324 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 4a0a49f2bb..5423978fed 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -370,341 +370,416 @@ static int rename_and_apply_smack(const char *temp_path, const char *dest_path) return r; } -static int write_files(void) { +static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char **tmpfile_path) { + _cleanup_fclose_ FILE *original = NULL, *passwd = NULL; + _cleanup_free_ char *passwd_tmp = NULL; + Iterator iterator; + Item *i; + int r; - _cleanup_fclose_ FILE *passwd = NULL, *group = NULL, *shadow = NULL, *gshadow = NULL; - _cleanup_free_ char *passwd_tmp = NULL, *group_tmp = NULL, *shadow_tmp = NULL, *gshadow_tmp = NULL; - const char *passwd_path = NULL, *group_path = NULL, *shadow_path = NULL, *gshadow_path = NULL; + if (hashmap_size(todo_uids) == 0) + return 0; + + r = fopen_temporary_label("/etc/passwd", passwd_path, &passwd, &passwd_tmp); + if (r < 0) + return r; + + original = fopen(passwd_path, "re"); + if (original) { + struct passwd *pw; + + r = sync_rights(original, passwd); + if (r < 0) + goto fail; + + errno = 0; + while ((pw = fgetpwent(original))) { + + i = hashmap_get(users, pw->pw_name); + if (i && i->todo_user) { + log_error("%s: User \"%s\" already exists.", passwd_path, pw->pw_name); + r = -EEXIST; + goto fail; + } + + if (hashmap_contains(todo_uids, UID_TO_PTR(pw->pw_uid))) { + log_error("%s: Detected collision for UID " UID_FMT ".", passwd_path, pw->pw_uid); + r = -EEXIST; + goto fail; + } + + errno = 0; + if (putpwent(pw, passwd) < 0) { + r = errno ? -errno : -EIO; + goto fail; + } + + errno = 0; + } + if (!IN_SET(errno, 0, ENOENT)) { + r = -errno; + goto fail; + } + + } else if (errno != ENOENT) { + r = -errno; + goto fail; + } else if (fchmod(fileno(passwd), 0644) < 0) { + r = -errno; + goto fail; + } + + HASHMAP_FOREACH(i, todo_uids, iterator) { + struct passwd n = { + .pw_name = i->name, + .pw_uid = i->uid, + .pw_gid = i->gid, + .pw_gecos = i->description, + + /* "x" means the password is stored in the shadow file */ + .pw_passwd = (char*) "x", + + /* We default to the root directory as home */ + .pw_dir = i->home ? i->home : (char*) "/", + + /* Initialize the shell to nologin, with one exception: + * for root we patch in something special */ + .pw_shell = i->uid == 0 ? (char*) "/bin/sh" : (char*) "/sbin/nologin", + }; + + errno = 0; + if (putpwent(&n, passwd) != 0) { + r = errno ? -errno : -EIO; + goto fail; + } + } + + r = fflush_and_check(passwd); + if (r < 0) + goto fail; + + *tmpfile = passwd; + *tmpfile_path = passwd_tmp; + passwd = NULL; + passwd_tmp = NULL; + return 0; + +fail: + unlink(passwd_tmp); + return r; +} + +static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char **tmpfile_path) { + _cleanup_fclose_ FILE *original = NULL, *shadow = NULL; + _cleanup_free_ char *shadow_tmp = NULL; + Iterator iterator; + long lstchg; + Item *i; + int r; + + if (hashmap_size(todo_uids) == 0) + return 0; + + r = fopen_temporary_label("/etc/shadow", shadow_path, &shadow, &shadow_tmp); + if (r < 0) + return r; + + lstchg = (long) (now(CLOCK_REALTIME) / USEC_PER_DAY); + + original = fopen(shadow_path, "re"); + if (original) { + struct spwd *sp; + + r = sync_rights(original, shadow); + if (r < 0) + goto fail; + + errno = 0; + while ((sp = fgetspent(original))) { + + i = hashmap_get(users, sp->sp_namp); + if (i && i->todo_user) { + /* we will update the existing entry */ + sp->sp_lstchg = lstchg; + + /* only the /etc/shadow stage is left, so we can + * safely remove the item from the todo set */ + i->todo_user = false; + hashmap_remove(todo_uids, UID_TO_PTR(i->uid)); + } + + errno = 0; + if (putspent(sp, shadow) < 0) { + r = errno ? -errno : -EIO; + goto fail; + } + + errno = 0; + } + if (!IN_SET(errno, 0, ENOENT)) { + r = -errno; + goto fail; + } + } else if (errno != ENOENT) { + r = -errno; + goto fail; + } else if (fchmod(fileno(shadow), 0000) < 0) { + r = -errno; + goto fail; + } + + HASHMAP_FOREACH(i, todo_uids, iterator) { + struct spwd n = { + .sp_namp = i->name, + .sp_pwdp = (char*) "!!", + .sp_lstchg = lstchg, + .sp_min = -1, + .sp_max = -1, + .sp_warn = -1, + .sp_inact = -1, + .sp_expire = -1, + .sp_flag = (unsigned long) -1, /* this appears to be what everybody does ... */ + }; + + errno = 0; + if (putspent(&n, shadow) != 0) { + r = errno ? -errno : -EIO; + goto fail; + } + } + + r = fflush_and_check(shadow); + if (r < 0) + goto fail; + + *tmpfile = shadow; + *tmpfile_path = shadow_tmp; + shadow = NULL; + shadow_tmp = NULL; + return 0; + +fail: + unlink(shadow_tmp); + return r; +} + +static int write_temporary_group(const char *group_path, FILE **tmpfile, char **tmpfile_path) { + _cleanup_fclose_ FILE *original = NULL, *group = NULL; + _cleanup_free_ char *group_tmp = NULL; bool group_changed = false; Iterator iterator; Item *i; int r; - if (hashmap_size(todo_gids) > 0 || hashmap_size(members) > 0) { - _cleanup_fclose_ FILE *original = NULL; + if (hashmap_size(todo_gids) == 0 && hashmap_size(members) == 0) + return 0; - /* First we update the actual group list file */ - group_path = prefix_roota(arg_root, "/etc/group"); - r = fopen_temporary_label("/etc/group", group_path, &group, &group_tmp); + r = fopen_temporary_label("/etc/group", group_path, &group, &group_tmp); + if (r < 0) + return r; + + original = fopen(group_path, "re"); + if (original) { + struct group *gr; + + r = sync_rights(original, group); if (r < 0) - goto finish; + goto fail; - original = fopen(group_path, "re"); - if (original) { - struct group *gr; + errno = 0; + while ((gr = fgetgrent(original))) { + /* Safety checks against name and GID collisions. Normally, + * this should be unnecessary, but given that we look at the + * entries anyway here, let's make an extra verification + * step that we don't generate duplicate entries. */ - r = sync_rights(original, group); + i = hashmap_get(groups, gr->gr_name); + if (i && i->todo_group) { + log_error("%s: Group \"%s\" already exists.", group_path, gr->gr_name); + r = -EEXIST; + goto fail; + } + + if (hashmap_contains(todo_gids, GID_TO_PTR(gr->gr_gid))) { + log_error("%s: Detected collision for GID " GID_FMT ".", group_path, gr->gr_gid); + r = -EEXIST; + goto fail; + } + + r = putgrent_with_members(gr, group); if (r < 0) - goto finish; + goto fail; + if (r > 0) + group_changed = true; errno = 0; - while ((gr = fgetgrent(original))) { - /* Safety checks against name and GID - * collisions. Normally, this should - * be unnecessary, but given that we - * look at the entries anyway here, - * let's make an extra verification - * step that we don't generate - * duplicate entries. */ - - i = hashmap_get(groups, gr->gr_name); - if (i && i->todo_group) { - log_error("%s: Group \"%s\" already exists.", group_path, gr->gr_name); - r = -EEXIST; - goto finish; - } - - if (hashmap_contains(todo_gids, GID_TO_PTR(gr->gr_gid))) { - log_error("%s: Detected collision for GID " GID_FMT ".", group_path, gr->gr_gid); - r = -EEXIST; - goto finish; - } - - r = putgrent_with_members(gr, group); - if (r < 0) - goto finish; - if (r > 0) - group_changed = true; - - errno = 0; - } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto finish; - } - - } else if (errno != ENOENT) { + } + if (!IN_SET(errno, 0, ENOENT)) { r = -errno; - goto finish; - } else if (fchmod(fileno(group), 0644) < 0) { - r = -errno; - goto finish; + goto fail; } - HASHMAP_FOREACH(i, todo_gids, iterator) { - struct group n = { - .gr_name = i->name, - .gr_gid = i->gid, - .gr_passwd = (char*) "x", - }; - - r = putgrent_with_members(&n, group); - if (r < 0) - goto finish; - - group_changed = true; - } - - r = fflush_and_check(group); - if (r < 0) - goto finish; - - if (original) { - fclose(original); - original = NULL; - } - - /* OK, now also update the shadow file for the group list */ - gshadow_path = prefix_roota(arg_root, "/etc/gshadow"); - r = fopen_temporary_label("/etc/gshadow", gshadow_path, &gshadow, &gshadow_tmp); - if (r < 0) - goto finish; - - original = fopen(gshadow_path, "re"); - if (original) { - struct sgrp *sg; - - r = sync_rights(original, gshadow); - if (r < 0) - goto finish; - - errno = 0; - while ((sg = fgetsgent(original))) { - - i = hashmap_get(groups, sg->sg_namp); - if (i && i->todo_group) { - log_error("%s: Group \"%s\" already exists.", gshadow_path, sg->sg_namp); - r = -EEXIST; - goto finish; - } - - r = putsgent_with_members(sg, gshadow); - if (r < 0) - goto finish; - if (r > 0) - group_changed = true; - - errno = 0; - } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto finish; - } - - } else if (errno != ENOENT) { - r = -errno; - goto finish; - } else if (fchmod(fileno(gshadow), 0000) < 0) { - r = -errno; - goto finish; - } - - HASHMAP_FOREACH(i, todo_gids, iterator) { - struct sgrp n = { - .sg_namp = i->name, - .sg_passwd = (char*) "!!", - }; - - r = putsgent_with_members(&n, gshadow); - if (r < 0) - goto finish; - - group_changed = true; - } - - r = fflush_and_check(gshadow); - if (r < 0) - goto finish; + } else if (errno != ENOENT) { + r = -errno; + goto fail; + } else if (fchmod(fileno(group), 0644) < 0) { + r = -errno; + goto fail; } - if (hashmap_size(todo_uids) > 0) { - _cleanup_fclose_ FILE *original = NULL; - long lstchg; + HASHMAP_FOREACH(i, todo_gids, iterator) { + struct group n = { + .gr_name = i->name, + .gr_gid = i->gid, + .gr_passwd = (char*) "x", + }; - /* First we update the user database itself */ - passwd_path = prefix_roota(arg_root, "/etc/passwd"); - r = fopen_temporary_label("/etc/passwd", passwd_path, &passwd, &passwd_tmp); + r = putgrent_with_members(&n, group); if (r < 0) - goto finish; + goto fail; - original = fopen(passwd_path, "re"); - if (original) { - struct passwd *pw; - - r = sync_rights(original, passwd); - if (r < 0) - goto finish; - - errno = 0; - while ((pw = fgetpwent(original))) { - - i = hashmap_get(users, pw->pw_name); - if (i && i->todo_user) { - log_error("%s: User \"%s\" already exists.", passwd_path, pw->pw_name); - r = -EEXIST; - goto finish; - } - - if (hashmap_contains(todo_uids, UID_TO_PTR(pw->pw_uid))) { - log_error("%s: Detected collision for UID " UID_FMT ".", passwd_path, pw->pw_uid); - r = -EEXIST; - goto finish; - } - - errno = 0; - if (putpwent(pw, passwd) < 0) { - r = errno ? -errno : -EIO; - goto finish; - } - - errno = 0; - } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto finish; - } - - } else if (errno != ENOENT) { - r = -errno; - goto finish; - } else if (fchmod(fileno(passwd), 0644) < 0) { - r = -errno; - goto finish; - } - - HASHMAP_FOREACH(i, todo_uids, iterator) { - struct passwd n = { - .pw_name = i->name, - .pw_uid = i->uid, - .pw_gid = i->gid, - .pw_gecos = i->description, - - /* "x" means the password is stored in - * the shadow file */ - .pw_passwd = (char*) "x", - - /* We default to the root directory as home */ - .pw_dir = i->home ? i->home : (char*) "/", - - /* Initialize the shell to nologin, - * with one exception: for root we - * patch in something special */ - .pw_shell = i->uid == 0 ? (char*) "/bin/sh" : (char*) "/sbin/nologin", - }; - - errno = 0; - if (putpwent(&n, passwd) != 0) { - r = errno ? -errno : -EIO; - goto finish; - } - } - - r = fflush_and_check(passwd); - if (r < 0) - goto finish; - - if (original) { - fclose(original); - original = NULL; - } - - /* The we update the shadow database */ - shadow_path = prefix_roota(arg_root, "/etc/shadow"); - r = fopen_temporary_label("/etc/shadow", shadow_path, &shadow, &shadow_tmp); - if (r < 0) - goto finish; - - lstchg = (long) (now(CLOCK_REALTIME) / USEC_PER_DAY); - - original = fopen(shadow_path, "re"); - if (original) { - struct spwd *sp; - - r = sync_rights(original, shadow); - if (r < 0) - goto finish; - - errno = 0; - while ((sp = fgetspent(original))) { - - i = hashmap_get(users, sp->sp_namp); - if (i && i->todo_user) { - /* we will update the existing entry */ - sp->sp_lstchg = lstchg; - - /* only the /etc/shadow stage is left, so we can - * safely remove the item from the todo set */ - i->todo_user = false; - hashmap_remove(todo_uids, UID_TO_PTR(i->uid)); - } - - errno = 0; - if (putspent(sp, shadow) < 0) { - r = errno ? -errno : -EIO; - goto finish; - } - - errno = 0; - } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto finish; - } - } else if (errno != ENOENT) { - r = -errno; - goto finish; - } else if (fchmod(fileno(shadow), 0000) < 0) { - r = -errno; - goto finish; - } - - HASHMAP_FOREACH(i, todo_uids, iterator) { - struct spwd n = { - .sp_namp = i->name, - .sp_pwdp = (char*) "!!", - .sp_lstchg = lstchg, - .sp_min = -1, - .sp_max = -1, - .sp_warn = -1, - .sp_inact = -1, - .sp_expire = -1, - .sp_flag = (unsigned long) -1, /* this appears to be what everybody does ... */ - }; - - errno = 0; - if (putspent(&n, shadow) != 0) { - r = errno ? -errno : -EIO; - goto finish; - } - } - - r = fflush_and_check(shadow); - if (r < 0) - goto finish; + group_changed = true; } + r = fflush_and_check(group); + if (r < 0) + goto fail; + + if (group_changed) { + *tmpfile = group; + *tmpfile_path = group_tmp; + group = NULL; + group_tmp = NULL; + } + return 0; + +fail: + unlink(group_tmp); + return r; +} + +static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, char **tmpfile_path) { + _cleanup_fclose_ FILE *original = NULL, *gshadow = NULL; + _cleanup_free_ char *gshadow_tmp = NULL; + bool group_changed = false; + Iterator iterator; + Item *i; + int r; + + if (hashmap_size(todo_gids) == 0 && hashmap_size(members) == 0) + return 0; + + r = fopen_temporary_label("/etc/gshadow", gshadow_path, &gshadow, &gshadow_tmp); + if (r < 0) + return r; + + original = fopen(gshadow_path, "re"); + if (original) { + struct sgrp *sg; + + r = sync_rights(original, gshadow); + if (r < 0) + goto fail; + + errno = 0; + while ((sg = fgetsgent(original))) { + + i = hashmap_get(groups, sg->sg_namp); + if (i && i->todo_group) { + log_error("%s: Group \"%s\" already exists.", gshadow_path, sg->sg_namp); + r = -EEXIST; + goto fail; + } + + r = putsgent_with_members(sg, gshadow); + if (r < 0) + goto fail; + if (r > 0) + group_changed = true; + + errno = 0; + } + if (!IN_SET(errno, 0, ENOENT)) { + r = -errno; + goto fail; + } + + } else if (errno != ENOENT) { + r = -errno; + goto fail; + } else if (fchmod(fileno(gshadow), 0000) < 0) { + r = -errno; + goto fail; + } + + HASHMAP_FOREACH(i, todo_gids, iterator) { + struct sgrp n = { + .sg_namp = i->name, + .sg_passwd = (char*) "!!", + }; + + r = putsgent_with_members(&n, gshadow); + if (r < 0) + goto fail; + + group_changed = true; + } + + r = fflush_and_check(gshadow); + if (r < 0) + goto fail; + + if (group_changed) { + *tmpfile = gshadow; + *tmpfile_path = gshadow_tmp; + gshadow = NULL; + gshadow_tmp = NULL; + } + return 0; + +fail: + unlink(gshadow_tmp); + return r; +} + +static int write_files(void) { + + _cleanup_fclose_ FILE *passwd = NULL, *group = NULL, *shadow = NULL, *gshadow = NULL; + _cleanup_free_ char *passwd_tmp = NULL, *group_tmp = NULL, *shadow_tmp = NULL, *gshadow_tmp = NULL; + const char *passwd_path = NULL, *group_path = NULL, *shadow_path = NULL, *gshadow_path = NULL; + int r; + + passwd_path = prefix_roota(arg_root, "/etc/passwd"); + shadow_path = prefix_roota(arg_root, "/etc/shadow"); + group_path = prefix_roota(arg_root, "/etc/group"); + gshadow_path = prefix_roota(arg_root, "/etc/gshadow"); + + r = write_temporary_group(group_path, &group, &group_tmp); + if (r < 0) + goto finish; + + r = write_temporary_gshadow(gshadow_path, &gshadow, &gshadow_tmp); + if (r < 0) + goto finish; + + r = write_temporary_passwd(passwd_path, &passwd, &passwd_tmp); + if (r < 0) + goto finish; + + r = write_temporary_shadow(shadow_path, &shadow, &shadow_tmp); + if (r < 0) + goto finish; + /* Make a backup of the old files */ - if (group_changed) { - if (group) { - r = make_backup("/etc/group", group_path); - if (r < 0) - goto finish; - } - if (gshadow) { - r = make_backup("/etc/gshadow", gshadow_path); - if (r < 0) - goto finish; - } + if (group) { + r = make_backup("/etc/group", group_path); + if (r < 0) + goto finish; + } + if (gshadow) { + r = make_backup("/etc/gshadow", gshadow_path); + if (r < 0) + goto finish; } if (passwd) { @@ -719,21 +794,19 @@ static int write_files(void) { } /* And make the new files count */ - if (group_changed) { - if (group) { - r = rename_and_apply_smack(group_tmp, group_path); - if (r < 0) - goto finish; + if (group) { + r = rename_and_apply_smack(group_tmp, group_path); + if (r < 0) + goto finish; - group_tmp = mfree(group_tmp); - } - if (gshadow) { - r = rename_and_apply_smack(gshadow_tmp, gshadow_path); - if (r < 0) - goto finish; + group_tmp = mfree(group_tmp); + } + if (gshadow) { + r = rename_and_apply_smack(gshadow_tmp, gshadow_path); + if (r < 0) + goto finish; - gshadow_tmp = mfree(gshadow_tmp); - } + gshadow_tmp = mfree(gshadow_tmp); } if (passwd) { From b14e1b43942f1f61146d9c6e519bd09688731797 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 9 May 2017 14:02:37 +0200 Subject: [PATCH 2/3] sysusers: make group shadow support configurable Some distros (openSUSE) don't have group shadow support enabled. This can lead to the following error: # systemd-sysusers Creating group foofoo with gid 478. # systemd-sysusers # groupdel foofoo # systemd-sysusers Creating group foofoo with gid 478. Failed to write files: File exists This patch adds --disable-gshadow option to configure. If used, systemd-sysvusers won't consider /etc/gshadow. --- configure.ac | 5 +++++ meson.build | 2 ++ meson_options.txt | 2 ++ src/sysusers/sysusers.c | 6 ++++++ 4 files changed, 15 insertions(+) diff --git a/configure.ac b/configure.ac index f59f3faf38..4851e5537d 100644 --- a/configure.ac +++ b/configure.ac @@ -1092,6 +1092,11 @@ if test "x$enable_sysusers" != "xno"; then fi AM_CONDITIONAL(ENABLE_SYSUSERS, [test "$have_sysusers" = "yes"]) +AC_ARG_ENABLE(gshadow, AS_HELP_STRING([--disable-gshadow], [disable shadow group support])) +AS_IF([test "x${enable_gshadow}" != "xno"], [ + AC_DEFINE(ENABLE_GSHADOW, 1, [shadow group support is enabled]) +]) + # ------------------------------------------------------------------------------ have_firstboot=no AC_ARG_ENABLE(firstboot, AS_HELP_STRING([--disable-firstboot], [disable firstboot support])) diff --git a/meson.build b/meson.build index 14a20530d4..1d842274f7 100644 --- a/meson.build +++ b/meson.build @@ -988,6 +988,7 @@ foreach pair : [['utmp', 'HAVE_UTMP'], ['tpm', 'SD_BOOT_LOG_TPM'], ['ima', 'HAVE_IMA'], ['smack', 'HAVE_SMACK'], + ['gshadow', 'ENABLE_GSHADOW'], ] if get_option(pair[0]) @@ -2473,6 +2474,7 @@ foreach tuple : [ ['hibernate'], ['adm group', get_option('adm-group')], ['wheel group', get_option('wheel-group')], + ['gshadow'], ['debug hashmap'], ['debug mmap cache'], ] diff --git a/meson_options.txt b/meson_options.txt index 4e99b25e63..3f55cdf1ff 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -146,6 +146,8 @@ option('dev-kvm-mode', type : 'string', value : '0660', description : '/dev/kvm access mode') option('default-kill-user-processes', type : 'boolean', description : 'the default value for KillUserProcesses= setting') +option('gshadow', type : 'boolean', + description : 'support for shadow group') option('default-dnssec', type : 'combo', description : 'default DNSSEC mode', diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 5423978fed..b8e8963feb 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -292,6 +292,7 @@ static int putgrent_with_members(const struct group *gr, FILE *group) { return 0; } +#ifdef ENABLE_GSHADOW static int putsgent_with_members(const struct sgrp *sg, FILE *gshadow) { char **a; @@ -341,6 +342,7 @@ static int putsgent_with_members(const struct sgrp *sg, FILE *gshadow) { return 0; } +#endif static int sync_rights(FILE *from, FILE *to) { struct stat st; @@ -659,6 +661,7 @@ fail: } static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, char **tmpfile_path) { +#ifdef ENABLE_GSHADOW _cleanup_fclose_ FILE *original = NULL, *gshadow = NULL; _cleanup_free_ char *gshadow_tmp = NULL; bool group_changed = false; @@ -740,6 +743,9 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch fail: unlink(gshadow_tmp); return r; +#else + return 0; +#endif } static int write_files(void) { From 1dd98a71e5b5e03e5baa232bdf63b7af71b53325 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 10 May 2017 14:28:41 +0200 Subject: [PATCH 3/3] sysusers: make use of cleanup(unlink_and_freep) in write_files() and its auxiliary helpers No functional changes. --- src/sysusers/sysusers.c | 198 +++++++++++++++------------------------- 1 file changed, 73 insertions(+), 125 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index b8e8963feb..fbe51a6ed5 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -29,6 +29,7 @@ #include "copy.h" #include "def.h" #include "fd-util.h" +#include "fs-util.h" #include "fileio-label.h" #include "format-util.h" #include "hashmap.h" @@ -374,7 +375,7 @@ static int rename_and_apply_smack(const char *temp_path, const char *dest_path) static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char **tmpfile_path) { _cleanup_fclose_ FILE *original = NULL, *passwd = NULL; - _cleanup_free_ char *passwd_tmp = NULL; + _cleanup_(unlink_and_freep) char *passwd_tmp = NULL; Iterator iterator; Item *i; int r; @@ -392,7 +393,7 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char r = sync_rights(original, passwd); if (r < 0) - goto fail; + return r; errno = 0; while ((pw = fgetpwent(original))) { @@ -400,35 +401,28 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char i = hashmap_get(users, pw->pw_name); if (i && i->todo_user) { log_error("%s: User \"%s\" already exists.", passwd_path, pw->pw_name); - r = -EEXIST; - goto fail; + return -EEXIST; } if (hashmap_contains(todo_uids, UID_TO_PTR(pw->pw_uid))) { log_error("%s: Detected collision for UID " UID_FMT ".", passwd_path, pw->pw_uid); - r = -EEXIST; - goto fail; + return -EEXIST; } errno = 0; - if (putpwent(pw, passwd) < 0) { - r = errno ? -errno : -EIO; - goto fail; - } + if (putpwent(pw, passwd) < 0) + return errno ? -errno : -EIO; errno = 0; } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto fail; - } + if (!IN_SET(errno, 0, ENOENT)) + return -errno; - } else if (errno != ENOENT) { - r = -errno; - goto fail; - } else if (fchmod(fileno(passwd), 0644) < 0) { - r = -errno; - goto fail; + } else { + if (errno != ENOENT) + return -errno; + if (fchmod(fileno(passwd), 0644) < 0) + return -errno; } HASHMAP_FOREACH(i, todo_uids, iterator) { @@ -450,30 +444,24 @@ static int write_temporary_passwd(const char *passwd_path, FILE **tmpfile, char }; errno = 0; - if (putpwent(&n, passwd) != 0) { - r = errno ? -errno : -EIO; - goto fail; - } + if (putpwent(&n, passwd) != 0) + return errno ? -errno : -EIO; } r = fflush_and_check(passwd); if (r < 0) - goto fail; + return r; *tmpfile = passwd; *tmpfile_path = passwd_tmp; passwd = NULL; passwd_tmp = NULL; return 0; - -fail: - unlink(passwd_tmp); - return r; } static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char **tmpfile_path) { _cleanup_fclose_ FILE *original = NULL, *shadow = NULL; - _cleanup_free_ char *shadow_tmp = NULL; + _cleanup_(unlink_and_freep) char *shadow_tmp = NULL; Iterator iterator; long lstchg; Item *i; @@ -494,7 +482,7 @@ static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char r = sync_rights(original, shadow); if (r < 0) - goto fail; + return r; errno = 0; while ((sp = fgetspent(original))) { @@ -511,23 +499,19 @@ static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char } errno = 0; - if (putspent(sp, shadow) < 0) { - r = errno ? -errno : -EIO; - goto fail; - } + if (putspent(sp, shadow) < 0) + return errno ? -errno : -EIO; errno = 0; } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto fail; - } - } else if (errno != ENOENT) { - r = -errno; - goto fail; - } else if (fchmod(fileno(shadow), 0000) < 0) { - r = -errno; - goto fail; + if (!IN_SET(errno, 0, ENOENT)) + return -errno; + + } else { + if (errno != ENOENT) + return -errno; + if (fchmod(fileno(shadow), 0000) < 0) + return -errno; } HASHMAP_FOREACH(i, todo_uids, iterator) { @@ -544,30 +528,24 @@ static int write_temporary_shadow(const char *shadow_path, FILE **tmpfile, char }; errno = 0; - if (putspent(&n, shadow) != 0) { - r = errno ? -errno : -EIO; - goto fail; - } + if (putspent(&n, shadow) != 0) + return errno ? -errno : -EIO; } r = fflush_and_check(shadow); if (r < 0) - goto fail; + return r; *tmpfile = shadow; *tmpfile_path = shadow_tmp; shadow = NULL; shadow_tmp = NULL; return 0; - -fail: - unlink(shadow_tmp); - return r; } static int write_temporary_group(const char *group_path, FILE **tmpfile, char **tmpfile_path) { _cleanup_fclose_ FILE *original = NULL, *group = NULL; - _cleanup_free_ char *group_tmp = NULL; + _cleanup_(unlink_and_freep) char *group_tmp = NULL; bool group_changed = false; Iterator iterator; Item *i; @@ -586,7 +564,7 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char ** r = sync_rights(original, group); if (r < 0) - goto fail; + return r; errno = 0; while ((gr = fgetgrent(original))) { @@ -598,35 +576,30 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char ** i = hashmap_get(groups, gr->gr_name); if (i && i->todo_group) { log_error("%s: Group \"%s\" already exists.", group_path, gr->gr_name); - r = -EEXIST; - goto fail; + return -EEXIST; } if (hashmap_contains(todo_gids, GID_TO_PTR(gr->gr_gid))) { log_error("%s: Detected collision for GID " GID_FMT ".", group_path, gr->gr_gid); - r = -EEXIST; - goto fail; + return -EEXIST; } r = putgrent_with_members(gr, group); if (r < 0) - goto fail; + return r; if (r > 0) group_changed = true; errno = 0; } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto fail; - } + if (!IN_SET(errno, 0, ENOENT)) + return -errno; - } else if (errno != ENOENT) { - r = -errno; - goto fail; - } else if (fchmod(fileno(group), 0644) < 0) { - r = -errno; - goto fail; + } else { + if (errno != ENOENT) + return -errno; + if (fchmod(fileno(group), 0644) < 0) + return -errno; } HASHMAP_FOREACH(i, todo_gids, iterator) { @@ -638,14 +611,14 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char ** r = putgrent_with_members(&n, group); if (r < 0) - goto fail; + return r; group_changed = true; } r = fflush_and_check(group); if (r < 0) - goto fail; + return r; if (group_changed) { *tmpfile = group; @@ -654,16 +627,12 @@ static int write_temporary_group(const char *group_path, FILE **tmpfile, char ** group_tmp = NULL; } return 0; - -fail: - unlink(group_tmp); - return r; } static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, char **tmpfile_path) { #ifdef ENABLE_GSHADOW _cleanup_fclose_ FILE *original = NULL, *gshadow = NULL; - _cleanup_free_ char *gshadow_tmp = NULL; + _cleanup_(unlink_and_freep) char *gshadow_tmp = NULL; bool group_changed = false; Iterator iterator; Item *i; @@ -682,7 +651,7 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch r = sync_rights(original, gshadow); if (r < 0) - goto fail; + return r; errno = 0; while ((sg = fgetsgent(original))) { @@ -690,29 +659,25 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch i = hashmap_get(groups, sg->sg_namp); if (i && i->todo_group) { log_error("%s: Group \"%s\" already exists.", gshadow_path, sg->sg_namp); - r = -EEXIST; - goto fail; + return -EEXIST; } r = putsgent_with_members(sg, gshadow); if (r < 0) - goto fail; + return r; if (r > 0) group_changed = true; errno = 0; } - if (!IN_SET(errno, 0, ENOENT)) { - r = -errno; - goto fail; - } + if (!IN_SET(errno, 0, ENOENT)) + return -errno; - } else if (errno != ENOENT) { - r = -errno; - goto fail; - } else if (fchmod(fileno(gshadow), 0000) < 0) { - r = -errno; - goto fail; + } else { + if (errno != ENOENT) + return -errno; + if (fchmod(fileno(gshadow), 0000) < 0) + return -errno; } HASHMAP_FOREACH(i, todo_gids, iterator) { @@ -723,14 +688,14 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch r = putsgent_with_members(&n, gshadow); if (r < 0) - goto fail; + return r; group_changed = true; } r = fflush_and_check(gshadow); if (r < 0) - goto fail; + return r; if (group_changed) { *tmpfile = gshadow; @@ -739,19 +704,14 @@ static int write_temporary_gshadow(const char * gshadow_path, FILE **tmpfile, ch gshadow_tmp = NULL; } return 0; - -fail: - unlink(gshadow_tmp); - return r; #else return 0; #endif } static int write_files(void) { - _cleanup_fclose_ FILE *passwd = NULL, *group = NULL, *shadow = NULL, *gshadow = NULL; - _cleanup_free_ char *passwd_tmp = NULL, *group_tmp = NULL, *shadow_tmp = NULL, *gshadow_tmp = NULL; + _cleanup_(unlink_and_freep) char *passwd_tmp = NULL, *group_tmp = NULL, *shadow_tmp = NULL, *gshadow_tmp = NULL; const char *passwd_path = NULL, *group_path = NULL, *shadow_path = NULL, *gshadow_path = NULL; int r; @@ -762,55 +722,55 @@ static int write_files(void) { r = write_temporary_group(group_path, &group, &group_tmp); if (r < 0) - goto finish; + return r; r = write_temporary_gshadow(gshadow_path, &gshadow, &gshadow_tmp); if (r < 0) - goto finish; + return r; r = write_temporary_passwd(passwd_path, &passwd, &passwd_tmp); if (r < 0) - goto finish; + return r; r = write_temporary_shadow(shadow_path, &shadow, &shadow_tmp); if (r < 0) - goto finish; + return r; /* Make a backup of the old files */ if (group) { r = make_backup("/etc/group", group_path); if (r < 0) - goto finish; + return r; } if (gshadow) { r = make_backup("/etc/gshadow", gshadow_path); if (r < 0) - goto finish; + return r; } if (passwd) { r = make_backup("/etc/passwd", passwd_path); if (r < 0) - goto finish; + return r; } if (shadow) { r = make_backup("/etc/shadow", shadow_path); if (r < 0) - goto finish; + return r; } /* And make the new files count */ if (group) { r = rename_and_apply_smack(group_tmp, group_path); if (r < 0) - goto finish; + return r; group_tmp = mfree(group_tmp); } if (gshadow) { r = rename_and_apply_smack(gshadow_tmp, gshadow_path); if (r < 0) - goto finish; + return r; gshadow_tmp = mfree(gshadow_tmp); } @@ -818,31 +778,19 @@ static int write_files(void) { if (passwd) { r = rename_and_apply_smack(passwd_tmp, passwd_path); if (r < 0) - goto finish; + return r; passwd_tmp = mfree(passwd_tmp); } if (shadow) { r = rename_and_apply_smack(shadow_tmp, shadow_path); if (r < 0) - goto finish; + return r; shadow_tmp = mfree(shadow_tmp); } - r = 0; - -finish: - if (passwd_tmp) - unlink(passwd_tmp); - if (shadow_tmp) - unlink(shadow_tmp); - if (group_tmp) - unlink(group_tmp); - if (gshadow_tmp) - unlink(gshadow_tmp); - - return r; + return 0; } static int uid_is_ok(uid_t uid, const char *name) {