URL: https://github.com/SSSD/sssd/pull/839 Author: alexey-tikhonov Title: #839: util/find_uid.c: fixed race condition bug Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/839/head:pr839 git checkout pr839
From 9539625a4d16a1c923b8d3c7fc1ee871afb152e2 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov <[email protected]> Date: Wed, 26 Jun 2019 16:20:19 +0200 Subject: [PATCH 1/2] util/find_uid.c: fixed debug message Fixed wrong debug message in check_if_uid_is_active() --- src/util/find_uid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/find_uid.c b/src/util/find_uid.c index 215c0d338e..9622b4728b 100644 --- a/src/util/find_uid.c +++ b/src/util/find_uid.c @@ -338,7 +338,7 @@ errno_t check_if_uid_is_active(uid_t uid, bool *result) ret = get_active_uid_linux(NULL, uid); if (ret != EOK && ret != ENOENT) { - DEBUG(SSSDBG_CRIT_FAILURE, "get_uid_table failed.\n"); + DEBUG(SSSDBG_CRIT_FAILURE, "get_active_uid_linux() failed.\n"); return ret; } From d8e6a81e5c0915b75b143c812e65d67038a776a8 Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov <[email protected]> Date: Wed, 26 Jun 2019 16:42:49 +0200 Subject: [PATCH 2/2] util/find_uid.c: fixed race condition bug It was wrong to return EOK from get_uid_from_pid() in case of failed open() or fstat() as this leaves `uid` uninitialized and no means for caller to detect this situation. There was no reason to fail get_active_uid_linux() completely in case of failed get_uid_from_pid() for one of /proc entries. Function was changed to continue with next entry instead. Resolves: https://pagure.io/SSSD/sssd/issue/2854 --- src/util/find_uid.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/util/find_uid.c b/src/util/find_uid.c index 9622b4728b..75ccadcf33 100644 --- a/src/util/find_uid.c +++ b/src/util/find_uid.c @@ -86,18 +86,17 @@ static errno_t get_uid_from_pid(const pid_t pid, uid_t *uid) error = errno; if (error == ENOENT) { DEBUG(SSSDBG_TRACE_LIBS, - "Proc file [%s] is not available anymore, continuing.\n", + "Proc file [%s] is not available anymore.\n", path); - return EOK; } else if (error == EPERM) { /* case of hidepid=1 mount option for /proc */ DEBUG(SSSDBG_TRACE_LIBS, - "Proc file [%s] is not permissible, continuing.\n", + "Proc file [%s] is not permissible.\n", path); - return EOK; + } else { + DEBUG(SSSDBG_CRIT_FAILURE, + "open failed [%s][%d][%s].\n", path, error, strerror(error)); } - DEBUG(SSSDBG_CRIT_FAILURE, - "open failed [%s][%d][%s].\n", path, error, strerror(error)); return error; } @@ -106,13 +105,12 @@ static errno_t get_uid_from_pid(const pid_t pid, uid_t *uid) error = errno; if (error == ENOENT) { DEBUG(SSSDBG_TRACE_LIBS, - "Proc file [%s] is not available anymore, continuing.\n", + "Proc file [%s] is not available anymore.\n", path); - error = EOK; - goto fail_fd; + } else { + DEBUG(SSSDBG_CRIT_FAILURE, + "fstat failed [%d][%s].\n", error, strerror(error)); } - DEBUG(SSSDBG_CRIT_FAILURE, - "fstat failed [%d][%s].\n", error, strerror(error)); goto fail_fd; } @@ -232,7 +230,9 @@ static errno_t get_active_uid_linux(hash_table_t *table, uid_t search_uid) errno = 0; while ((dirent = readdir(proc_dir)) != NULL) { - if (only_numbers(dirent->d_name) != 0) continue; + if (only_numbers(dirent->d_name) != 0) { + continue; + } ret = name_to_pid(dirent->d_name, &pid); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "name_to_pid failed.\n"); @@ -241,8 +241,12 @@ static errno_t get_active_uid_linux(hash_table_t *table, uid_t search_uid) ret = get_uid_from_pid(pid, &uid); if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "get_uid_from_pid failed.\n"); - goto done; + /* Most probably this /proc entry disappeared. + Anyway, just skip it. + */ + DEBUG(SSSDBG_TRACE_ALL, "get_uid_from_pid() failed.\n"); + errno = 0; + continue; } if (table != NULL) { @@ -265,10 +269,9 @@ static errno_t get_active_uid_linux(hash_table_t *table, uid_t search_uid) } } - errno = 0; } - if (errno != 0 && dirent == NULL) { + if (errno != 0) { ret = errno; DEBUG(SSSDBG_CRIT_FAILURE, "readdir failed.\n"); goto done;
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/[email protected]
