URL: https://github.com/SSSD/sssd/pull/839
Author: alexey-tikhonov
Title: #839: util/find_uid.c: fixed race condition bug
Action: opened
PR body:
"""
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
"""
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 bd82bbbb4360ba742265b61cacc8868a800539b8 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 | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/util/find_uid.c b/src/util/find_uid.c
index 9622b4728b..3c27210911 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,11 @@ 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");
+ continue;
}
if (table != NULL) {
_______________________________________________
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]
