From 0241c6f1df5068c006f756005c8e7faa63058c27 Mon Sep 17 00:00:00 2001 From: Ken'ichi Ohmichi Date: Tue, 19 May 2009 09:19:29 +0900 Subject: [PATCH] Fix the deadlock of rl_lock. Hi, Changelog of v2: - Add the description of the problematic call sequence. - There is not any change in the code. [PATCH-v2] Fix the deadlock of rl_lock. For avoiding the deadlock, protect cdgroup_change_cgroup_uid_gid_flags() by blocking SIGUSR2 signal. The problematic call sequence is the following: ---------------------------------------------------------------------- * CGRULESENGD DAEMON * << cgre_flash_rules() is the signal handler for SIGUSR2 signal >> cgre_create_netlink_socket_process_msg() << Receive a UID/GID event packet >> cgre_handle_msg() cgre_process_event() cgroup_change_cgroup_uid_gid_flags() cgroup_find_matching_rule_uid_gid() pthread_rwlock_wrlock(&rl_lock); << Get the lock of rl_lock >> << Receive a SIGUSR2 signal, and switch to cgre_flash_rules() >> cgre_flash_rules() cgroup_reload_cached_rules() cgroup_parse_rules() pthread_rwlock_wrlock(&rl_lock); << deadlock ! >> ---------------------------------------------------------------------- A cgrulesengd daemon needs a lock of rl_lock for referring configuration buffer. On the other way, the daemon reloads configuration file when receiving SIGUSR2 signal, and it needs the same lock in cgroup_parse_rules(). So cgroup_change_cgroup_uid_gid_flags() should be protected from SIGUSR2 signal for avoiding the deadlock. Thanks Ken'ichi Ohmichi Signed-off-by: Ken'ichi Ohmichi Signed-off-by: Dhaval Giani --- src/daemon/cgrulesengd.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c index 1a614762..fb757c14 100644 --- a/src/daemon/cgrulesengd.c +++ b/src/daemon/cgrulesengd.c @@ -271,6 +271,27 @@ static int cgre_was_parent_changed_when_forking(const struct proc_event *ev) return 0; } +static int cgre_change_cgroup_uid_gid(const uid_t uid, const gid_t gid, + const pid_t pid) +{ + int ret; + sigset_t sigset; + + /* + * For avoiding the deadlock, protect cdgroup_change_cgroup_ + * ~uid_gid_flags() by blocking SIGUSR2 signal. + */ + sigemptyset(&sigset); + sigaddset(&sigset, SIGUSR2); + sigprocmask(SIG_BLOCK, &sigset, NULL); + + ret = cgroup_change_cgroup_uid_gid_flags(uid, gid, pid, + CGFLAG_USECACHE); + sigprocmask(SIG_UNBLOCK, &sigset, NULL); + + return ret; +} + /** * 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 @@ -318,22 +339,20 @@ int cgre_process_event(const struct proc_event *ev, const int type) case PROC_EVENT_UID: log_uid = ev->event_data.id.e.euid; log_gid = egid; - ret = cgroup_change_cgroup_uid_gid_flags( + ret = cgre_change_cgroup_uid_gid( ev->event_data.id.e.euid, - egid, pid, CGFLAG_USECACHE); + egid, pid); break; case PROC_EVENT_GID: log_uid = euid; log_gid = ev->event_data.id.e.egid; - ret = cgroup_change_cgroup_uid_gid_flags(euid, - ev->event_data.id.e.egid, - pid, CGFLAG_USECACHE); + ret = cgre_change_cgroup_uid_gid(euid, + ev->event_data.id.e.egid, pid); break; case PROC_EVENT_FORK: log_uid = euid; log_gid = egid; - ret = cgroup_change_cgroup_uid_gid_flags(euid, - egid, pid, CGFLAG_USECACHE); + ret = cgre_change_cgroup_uid_gid(euid, egid, pid); break; default: break; -- 2.47.2