diff options
author | \"Ken'ichi Ohmichi\ <oomichi@mxs.nes.nec.co.jp> | 2009-05-08 01:27:46 +0530 |
---|---|---|
committer | Balbir Singh <balbir@linux.vnet.ibm.com> | 2009-05-08 01:27:46 +0530 |
commit | 485fe65a0daa23899d1e4c02620036a265560a57 (patch) | |
tree | 081e1c553d3eef4e38138b704e69511eb3b97826 | |
parent | 739cdfd62e14d2558566cf73ce9e1702929cf834 (diff) | |
download | libcg-485fe65a0daa23899d1e4c02620036a265560a57.tar.gz libcg-485fe65a0daa23899d1e4c02620036a265560a57.tar.xz libcg-485fe65a0daa23899d1e4c02620036a265560a57.zip |
I have been testing a cgrulesengd daemon and I noticed it fails to
change the cgroup of child occasionally. I tested it by following
configulation file:
/etc/cgrules.conf:
user01 cpuset group01/user01
% memory group01/user01
A cpuset subsystem and a memory subsystem are mounted on different
mount points, and a cgrulesengd daemon manages each subsystem.
I login this environment as a user "user01", and each susbystem's
tasks file is the following:
# cat /mnt/cgroups/cpuset/group01/user01/tasks
31801
31805
31806
#
# cat /mnt/cgroups/memory/group01/user01/tasks
31801
31805
#
# pstree -p 32105
sshd(31801)---sshd(31805)---bash(31806)
#
They should be the same, but they are different. I investigated this
problem, and I found the cause. The reason is that the process(31806)
was forked just after writing the process(31805) to a cpuset subsystem's
tasks file:
<1> The UID/GID CHANGE event of the process 31805 happens.
<2> The daemon writes "31805" to a cpuset subsystem's tasks file.
<3> The process 31806 is forked, and it appears on a cpuset subsystem's
tasks file.
<4> The daemon writes "31805" to a memory subsystem's tasks file.
<5> The process 31806 does not appears on a memory subsystem's tasks file.
For solving this problem, I propose the following sequence.
1. Store both the timestamp and the process-id when the step <4>.
2. If receiving a PROC_EVENT_FORK packet, check its parent-pid and its
timestamp.
3. If its parent-pid and the stored process-id are same and its timestamp
is older than the stored timestamp, change the cgroup of forked process.
Changelog of v2:
* Change only [PATCH 2/2] and there is not any changes in [PATCH 1/2].
This patch adds the method for getting euid/egid from /proc/<pid>/status
file.
For changing the cgroup of a forked process, the method is usefull because
a PROC_EVENT_FORK packet does not inform of its euid and its egid.
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
-rw-r--r-- | src/daemon/cgrulesengd.c | 107 |
1 files changed, 47 insertions, 60 deletions
diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c index 4e00e18..74d12e1 100644 --- a/src/daemon/cgrulesengd.c +++ b/src/daemon/cgrulesengd.c @@ -132,15 +132,7 @@ void flog(int level, const char *format, ...) } } -/** - * Process an event from the kernel, and determine the correct UID/GID/PID to - * pass to libcgroup. Then, libcgroup will decide the cgroup to move the PID - * to, if any. - * @param ev The event to process - * @param type The type of event to process (part of ev) - * @return 0 on success, > 0 on failure - */ -int cgre_process_event(const struct proc_event *ev, const int type) +static int cgre_get_euid_egid_from_status(pid_t pid, uid_t *euid, gid_t *egid) { /* Handle for the /proc/PID/status file */ FILE *f; @@ -149,19 +141,13 @@ int cgre_process_event(const struct proc_event *ev, const int type) char path[FILENAME_MAX]; /* Temporary buffer */ - char *buf = NULL; + char buf[4092]; /* UID data */ - uid_t ruid, euid, suid, fsuid, log_uid = 0; + uid_t ruid, suid, fsuid; /* GID data */ - gid_t rgid, egid, sgid, fsgid, log_gid = 0; - - /* PID, just for logging */ - pid_t log_pid = 0; - - /* Return codes */ - int ret = 0; + gid_t rgid, sgid, fsgid; /* * First, we need to open the /proc/PID/status file so that we can @@ -169,76 +155,79 @@ int cgre_process_event(const struct proc_event *ev, const int type) * on. This process is probably not us, so we can't just call * geteuid() or getegid(). */ - sprintf(path, "/proc/%d/status", ev->event_data.id.process_pid); + sprintf(path, "/proc/%d/status", pid); f = fopen(path, "r"); if (!f) { flog(LOG_WARNING, "Failed to open %s", path); - goto finished; + return 1; } - /* Now, we need to find either the eUID or the eGID of the process. */ - buf = calloc(4096, sizeof(char)); - if (!buf) { - flog(LOG_WARNING, "Failed to process event, out of" - "memory? Error: %s", - strerror(errno)); - ret = errno; - fclose(f); - goto finished; + /* Have the eUID, need to find the eGID. */ + memset(buf, '\0', sizeof(buf)); + while (fgets(buf, sizeof(buf), f)) { + if (!strncmp(buf, "Uid:", 4)) { + sscanf((buf + 5), "%d%d%d%d", &ruid, euid, + &suid, &fsuid); + break; + } else if (!strncmp(buf, "Gid:", 4)) { + sscanf((buf + 5), "%d%d%d%d", &rgid, egid, + &sgid, &fsgid); + break; + } + memset(buf, '\0', sizeof(buf)); } + fclose(f); + return 0; +} + +/** + * Process an event from the kernel, and determine the correct UID/GID/PID to + * pass to libcgroup. Then, libcgroup will decide the cgroup to move the PID + * to, if any. + * @param ev The event to process + * @param type The type of event to process (part of ev) + * @return 0 on success, > 0 on failure + */ +int cgre_process_event(const struct proc_event *ev, const int type) +{ + pid_t pid = 0, log_pid = 0; + uid_t euid, log_uid = 0; + gid_t egid, log_gid = 0; + + int ret = 0; + switch (type) { case PROC_EVENT_UID: - /* Have the eUID, need to find the eGID. */ - while (fgets(buf, 4096, f)) { - if (!strncmp(buf, "Gid:", 4)) { - sscanf((buf + 5), "%d%d%d%d", &rgid, &egid, - &sgid, &fsgid); - break; - } - memset(buf, '\0', 4096); - } - break; case PROC_EVENT_GID: - /* Have the eGID, need to find the eUID. */ - while (fgets(buf, 4096, f)) { - if (!strncmp(buf, "Uid:", 4)) { - sscanf((buf + 5), "%d%d%d%d", &ruid, &euid, - &suid, &fsuid); - break; - } - memset(buf, '\0', 4096); - } + pid = ev->event_data.id.process_pid; break; default: - flog(LOG_WARNING, "For some reason, we're processing a" - " non-UID/GID event. Something is wrong!"); break; } - free(buf); - fclose(f); + if (cgre_get_euid_egid_from_status(pid, &euid, &egid)) + /* cgre_get_euid_egid_from_status() returns 1 if it fails to + * open /proc/<pid>/status file and that is not a problem. */ + return 0; /* * Now that we have the UID, the GID, and the PID, we can make a call * to libcgroup to change the cgroup for this PID. */ + log_pid = pid; switch (type) { case PROC_EVENT_UID: log_uid = ev->event_data.id.e.euid; log_gid = egid; - log_pid = ev->event_data.id.process_pid; ret = cgroup_change_cgroup_uid_gid_flags( ev->event_data.id.e.euid, - egid, ev->event_data.id.process_pid, - CGFLAG_USECACHE); + egid, pid, CGFLAG_USECACHE); break; case PROC_EVENT_GID: log_uid = euid; log_gid = ev->event_data.id.e.egid; - log_pid = ev->event_data.id.process_pid; ret = cgroup_change_cgroup_uid_gid_flags(euid, ev->event_data.id.e.egid, - ev->event_data.id.process_pid, - CGFLAG_USECACHE); + pid, CGFLAG_USECACHE); break; default: break; @@ -256,8 +245,6 @@ int cgre_process_event(const struct proc_event *ev, const int type) flog(LOG_INFO, "Cgroup change for PID: %d, UID: %d, GID: %d OK", log_pid, log_uid, log_gid); } - -finished: return ret; } |