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>

diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c
index 1a61476253ebf8452f669e032375ab6e1ec54b04..fb757c1443748b207c4ade30f73c4a9ca8c4cd4e 100644 (file)
--- 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;