fix the deadlock in xt_qtaguid when enable DDEBUG
authorChenbo Feng <fengc@google.com>
Thu, 23 Mar 2017 20:51:24 +0000 (13:51 -0700)
committerAmit Pundir <amit.pundir@linaro.org>
Mon, 10 Apr 2017 07:42:16 +0000 (13:12 +0530)
When DDEBUG is enabled, the prdebug_full_state() function will try to
recursively aquire the spinlock of sock_tag_list and causing deadlock. A
check statement is added before it aquire the spinlock to differentiate
the behavior depend on the caller of the function.

Bug: 36559739
Test: Compile and run test under system/extra/test/iptables/
Change-Id: Ie3397fbaa207e14fe214d47aaf5e8ca1f4a712ee
Signed-off-by: Chenbo Feng <fengc@google.com>
net/netfilter/xt_qtaguid.c

index 3bf0c59dab2f54c50065b47632cbbbe31b1ff043..0f5628a59917e393f0eec7cb564a16dd6e1c0b22 100644 (file)
@@ -1814,8 +1814,11 @@ ret_res:
 }
 
 #ifdef DDEBUG
-/* This function is not in xt_qtaguid_print.c because of locks visibility */
-static void prdebug_full_state(int indent_level, const char *fmt, ...)
+/*
+ * This function is not in xt_qtaguid_print.c because of locks visibility.
+ * The lock of sock_tag_list must be aquired before calling this function
+ */
+static void prdebug_full_state_locked(int indent_level, const char *fmt, ...)
 {
        va_list args;
        char *fmt_buff;
@@ -1836,16 +1839,12 @@ static void prdebug_full_state(int indent_level, const char *fmt, ...)
        kfree(buff);
        va_end(args);
 
-       spin_lock_bh(&sock_tag_list_lock);
        prdebug_sock_tag_tree(indent_level, &sock_tag_tree);
-       spin_unlock_bh(&sock_tag_list_lock);
 
-       spin_lock_bh(&sock_tag_list_lock);
        spin_lock_bh(&uid_tag_data_tree_lock);
        prdebug_uid_tag_data_tree(indent_level, &uid_tag_data_tree);
        prdebug_proc_qtu_data_tree(indent_level, &proc_qtu_data_tree);
        spin_unlock_bh(&uid_tag_data_tree_lock);
-       spin_unlock_bh(&sock_tag_list_lock);
 
        spin_lock_bh(&iface_stat_list_lock);
        prdebug_iface_stat_list(indent_level, &iface_stat_list);
@@ -1854,7 +1853,7 @@ static void prdebug_full_state(int indent_level, const char *fmt, ...)
        pr_debug("qtaguid: %s(): }\n", __func__);
 }
 #else
-static void prdebug_full_state(int indent_level, const char *fmt, ...) {}
+static void prdebug_full_state_locked(int indent_level, const char *fmt, ...) {}
 #endif
 
 struct proc_ctrl_print_info {
@@ -1977,8 +1976,11 @@ static int qtaguid_ctrl_proc_show(struct seq_file *m, void *v)
                           (u64)atomic64_read(&qtu_events.match_no_sk),
                           (u64)atomic64_read(&qtu_events.match_no_sk_file));
 
-               /* Count the following as part of the last item_index */
-               prdebug_full_state(0, "proc ctrl");
+               /* Count the following as part of the last item_index. No need
+                * to lock the sock_tag_list here since it is already locked when
+                * starting the seq_file operation
+                */
+               prdebug_full_state_locked(0, "proc ctrl");
        }
 
        return 0;
@@ -2887,8 +2889,10 @@ static int qtudev_release(struct inode *inode, struct file *file)
 
        sock_tag_tree_erase(&st_to_free_tree);
 
-       prdebug_full_state(0, "%s(): pid=%u tgid=%u", __func__,
+       spin_lock_bh(&sock_tag_list_lock);
+       prdebug_full_state_locked(0, "%s(): pid=%u tgid=%u", __func__,
                           current->pid, current->tgid);
+       spin_unlock_bh(&sock_tag_list_lock);
        return 0;
 }