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 <oomichi@mxs.nes.nec.co.jp>
Signed-off-by: Dhaval Giani <dhaval@linux.vnet.ibm.com>

1 files changed, 26 insertions, 7 deletions

diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c
index 1a61476..fb757c1 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;