netfilter: xt_qtaguid: fix crash after using delete ctrl command

* Crash fix
The delete command would delete a socket tag entry without removing it
from the proc_qtu_data { ..., sock_tag_list, }.
This in turn would cause an exiting process to crash while cleaning up
its matching proc_qtu_data.

* Added more aggressive tracking/cleanup of proc_qtu_data
This should allow one process to cleanup qtu_tag_data{} left around from
processes that didn't use resource tracking via /dev/xt_qtaguid.

* Debug printing tweaks
Better code inclusion/exclusion handling,
and extra debug out of full state.

Change-Id: I735965af2962ffcd7f3021cdc0068b3ab21245c2
Signed-off-by: JP Abgrall <jpa@google.com>

4 files changed, 449 insertions, 196 deletions

diff --git a/net/netfilter/xt_qtaguid.c b/net/netfilter/xt_qtaguid.c
index c42e486c5cbe..32d855b1b6d2 100644
--- a/net/netfilter/xt_qtaguid.c
+++ b/net/netfilter/xt_qtaguid.c
@@ -102,11 +102,10 @@ static bool qtu_proc_handling_passive;
 module_param_named(tag_tracking_passive, qtu_proc_handling_passive, bool,
 		   S_IRUGO | S_IWUSR);
 
-
 #define QTU_DEV_NAME "xt_qtaguid"
 
-uint debug_mask = DEFAULT_DEBUG_MASK;
-module_param(debug_mask, uint, S_IRUGO | S_IWUSR);
+uint qtaguid_debug_mask = DEFAULT_DEBUG_MASK;
+module_param_named(debug_mask, qtaguid_debug_mask, uint, S_IRUGO | S_IWUSR);
 
 /*---------------------------------------------------------------------------*/
 static const char *iface_stat_procdirname = "iface_stat";
@@ -125,70 +124,92 @@ static struct proc_dir_entry *iface_stat_all_procfile;
  * Notice how sock_tag_list_lock is held sometimes when uid_tag_data_tree_lock
  * is acquired.
  *
- * Call tree with all lock holders as of 2011-09-06:
- *
- *   qtaguid_ctrl_parse()
- *     ctrl_cmd_delete()
- *       sock_tag_list_lock
- *       tag_counter_set_list_lock
- *       iface_stat_list_lock
- *         iface_entry->tag_stat_list_lock
- *       uid_tag_data_tree_lock
- *     ctrl_cmd_counter_set()
- *       tag_counter_set_list_lock
- *     ctrl_cmd_tag()
- *       sock_tag_list_lock
- *         get_tag_ref()
- *           uid_tag_data_tree_lock
- *       uid_tag_data_tree_lock
- *     ctrl_cmd_untag()
- *       sock_tag_list_lock
- *         uid_tag_data_tree_lock
+ * Call tree with all lock holders as of 2011-09-25:
  *
- *   qtaguid_mt()
- *     account_for_uid()
- *       if_tag_stat_update()
- *     get_sock_stat()
- *       sock_tag_list_lock
- *        iface_entry->tag_stat_list_lock
- *        tag_stat_update()
- *          get_active_counter_set()
- *            tag_counter_set_list_lock
+ * iface_stat_all_proc_read()
+ *   iface_stat_list_lock
+ *     (struct iface_stat)
  *
- *   iface_netdev_event_handler()
- *     iface_stat_create()
- *       iface_stat_list_lock
- *     iface_stat_update()
- *       iface_stat_list_lock
+ * qtaguid_ctrl_proc_read()
+ *   sock_tag_list_lock
+ *     (sock_tag_tree)
+ *     (struct proc_qtu_data->sock_tag_list)
+ *   prdebug_full_state()
+ *     sock_tag_list_lock
+ *       (sock_tag_tree)
+ *     uid_tag_data_tree_lock
+ *       (uid_tag_data_tree)
+ *       (proc_qtu_data_tree)
+ *     iface_stat_list_lock
  *
- *   iface_inet6addr_event_handler()
- *     iface_stat_create_ipv6()
- *       iface_stat_list_lock
- *     iface_stat_update()
- *       iface_stat_list_lock
+ * qtaguid_stats_proc_read()
+ *   iface_stat_list_lock
+ *     struct iface_stat->tag_stat_list_lock
  *
- *   iface_inetaddr_event_handler()
- *     iface_stat_create()
- *       iface_stat_list_lock
- *     iface_stat_update()
- *       iface_stat_list_lock
+ * qtudev_open()
+ *   uid_tag_data_tree_lock
  *
- *   qtaguid_ctrl_proc_read()
- *     sock_tag_list_lock
+ * qtudev_release()
+ *   sock_tag_data_list_lock
+ *     uid_tag_data_tree_lock
+ *   prdebug_full_state()
  *     sock_tag_list_lock
  *     uid_tag_data_tree_lock
  *     iface_stat_list_lock
  *
- *   qtaguid_stats_proc_read()
+ * iface_netdev_event_handler()
+ *   iface_stat_create()
+ *     iface_stat_list_lock
+ *   iface_stat_update()
  *     iface_stat_list_lock
- *       iface_entry->tag_stat_list_lock
  *
- *   qtudev_open()
- *     uid_tag_data_tree_lock
+ * iface_inetaddr_event_handler()
+ *   iface_stat_create()
+ *     iface_stat_list_lock
+ *   iface_stat_update()
+ *     iface_stat_list_lock
+ *
+ * iface_inet6addr_event_handler()
+ *   iface_stat_create_ipv6()
+ *     iface_stat_list_lock
+ *   iface_stat_update()
+ *     iface_stat_list_lock
  *
- *   qtud_dev_release()
+ * qtaguid_mt()
+ *   account_for_uid()
+ *     if_tag_stat_update()
+ *       get_sock_stat()
+ *         sock_tag_list_lock
+ *       struct iface_stat->tag_stat_list_lock
+ *         tag_stat_update()
+ *           get_active_counter_set()
+ *             tag_counter_set_list_lock
+ *         tag_stat_update()
+ *           get_active_counter_set()
+ *             tag_counter_set_list_lock
+ *
+ *
+ * qtaguid_ctrl_parse()
+ *   ctrl_cmd_delete()
+ *     sock_tag_list_lock
+ *     tag_counter_set_list_lock
+ *     iface_stat_list_lock
+ *       struct iface_stat->tag_stat_list_lock
+ *     uid_tag_data_tree_lock
+ *   ctrl_cmd_counter_set()
+ *     tag_counter_set_list_lock
+ *   ctrl_cmd_tag()
  *     sock_tag_list_lock
+ *       (sock_tag_tree)
+ *       get_tag_ref()
+ *         uid_tag_data_tree_lock
+ *           (uid_tag_data_tree)
  *       uid_tag_data_tree_lock
+ *         (proc_qtu_data_tree)
+ *   ctrl_cmd_untag()
+ *     sock_tag_list_lock
+ *     uid_tag_data_tree_lock
+ *
  */
 static LIST_HEAD(iface_stat_list);
 static DEFINE_SPINLOCK(iface_stat_list_lock);
@@ -557,8 +578,8 @@ static struct tag_ref *new_tag_ref(tag_t new_tag,
 	utd_entry->num_active_tags++;
 	tag_ref_tree_insert(tr_entry, &utd_entry->tag_ref_tree);
 	DR_DEBUG("qtaguid: new_tag_ref(0x%llx): "
-		 " inserted new tag ref\n",
-		 new_tag);
+		 " inserted new tag ref %p\n",
+		 new_tag, tr_entry);
 	return tr_entry;
 
 err_res:
@@ -618,7 +639,8 @@ static struct tag_ref *get_tag_ref(tag_t full_tag,
 static void put_utd_entry(struct uid_tag_data *utd_entry)
 {
 	/* Are we done with the UID tag data entry? */
-	if (RB_EMPTY_ROOT(&utd_entry->tag_ref_tree)) {
+	if (RB_EMPTY_ROOT(&utd_entry->tag_ref_tree) &&
+		!utd_entry->num_pqd) {
 		DR_DEBUG("qtaguid: %s(): "
 			 "erase utd_entry=%p uid=%u "
 			 "by pid=%u tgid=%u uid=%u\n", __func__,
@@ -629,9 +651,11 @@ static void put_utd_entry(struct uid_tag_data *utd_entry)
 		kfree(utd_entry);
 	} else {
 		DR_DEBUG("qtaguid: %s(): "
-			 "utd_entry=%p still has %d tags\n", __func__,
-			 utd_entry, utd_entry->num_active_tags);
-		BUG_ON(!utd_entry->num_active_tags);
+			 "utd_entry=%p still has %d tags %d proc_qtu_data\n",
+			 __func__, utd_entry, utd_entry->num_active_tags,
+			 utd_entry->num_pqd);
+		BUG_ON(!(utd_entry->num_active_tags ||
+			 utd_entry->num_pqd));
 	}
 }
 
@@ -1309,8 +1333,8 @@ static void if_tag_stat_update(const char *ifname, uid_t uid,
 		new_tag_stat = create_if_tag_stat(iface_entry, tag);
 		new_tag_stat->parent_counters = uid_tag_counters;
 	}
-	spin_unlock_bh(&iface_entry->tag_stat_list_lock);
 	tag_stat_update(new_tag_stat, direction, proto, bytes);
+	spin_unlock_bh(&iface_entry->tag_stat_list_lock);
 }
 
 static int iface_netdev_event_handler(struct notifier_block *nb,
@@ -1672,6 +1696,50 @@ ret_res:
 	return 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, ...)
+{
+	va_list args;
+	char *fmt_buff;
+	char *buff;
+
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	fmt_buff = kasprintf(GFP_ATOMIC,
+			     "qtaguid: %s(): %s {\n", __func__, fmt);
+	BUG_ON(!fmt_buff);
+	va_start(args, fmt);
+	buff = kvasprintf(GFP_ATOMIC,
+			  fmt_buff, args);
+	BUG_ON(!buff);
+	pr_debug("%s", buff);
+	kfree(fmt_buff);
+	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);
+	spin_unlock_bh(&iface_stat_list_lock);
+
+	pr_debug("qtaguid: %s(): }\n", __func__);
+}
+#else
+static void prdebug_full_state(int indent_level, const char *fmt, ...) {}
+#endif
+
 /*
  * Procfs reader to get all active socket tags using style "1)" as described in
  * fs/proc/generic.c
@@ -1761,28 +1829,10 @@ static int qtaguid_ctrl_proc_read(char *page, char **num_items_returned,
 		(*num_items_returned)++;
 	}
 
-#ifdef CDEBUG
 	/* Count the following as part of the last item_index */
 	if (item_index > items_to_skip) {
-		CT_DEBUG("qtaguid: proc ctrl state debug {\n");
-		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(&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_lock_bh(&iface_stat_list_lock);
-		prdebug_iface_stat_list(indent_level, &iface_stat_list);
-		spin_unlock_bh(&iface_stat_list_lock);
-
-		CT_DEBUG("qtaguid: proc ctrl state debug }\n");
-
-
+		prdebug_full_state(indent_level, "proc ctrl");
 	}
-#endif
 
 	*eof = 1;
 	return outp - page;
@@ -1833,9 +1883,9 @@ static int ctrl_cmd_delete(const char *input)
 	}
 
 	tag = combine_atag_with_uid(acct_tag, uid);
-	CT_DEBUG("qtaguid: ctrl_delete(): "
+	CT_DEBUG("qtaguid: ctrl_delete(%s): "
 		 "looking for tag=0x%llx (uid=%u)\n",
-		 tag, uid);
+		 input, tag, uid);
 
 	/* Delete socket tags */
 	spin_lock_bh(&sock_tag_list_lock);
@@ -1847,8 +1897,8 @@ static int ctrl_cmd_delete(const char *input)
 		if (entry_uid != uid)
 			continue;
 
-		CT_DEBUG("qtaguid: ctrl_delete(): st tag=0x%llx (uid=%u)\n",
-			 st_entry->tag, entry_uid);
+		CT_DEBUG("qtaguid: ctrl_delete(%s): st tag=0x%llx (uid=%u)\n",
+			 input, st_entry->tag, entry_uid);
 
 		if (!acct_tag || st_entry->tag == tag) {
 			rb_erase(&st_entry->sock_node, &sock_tag_tree);
@@ -1857,6 +1907,7 @@ static int ctrl_cmd_delete(const char *input)
 			tr_entry = lookup_tag_ref(st_entry->tag, NULL);
 			BUG_ON(tr_entry->num_sock_tags <= 0);
 			tr_entry->num_sock_tags--;
+			list_del(&st_entry->list);
 		}
 	}
 	spin_unlock_bh(&sock_tag_list_lock);
@@ -1868,8 +1919,9 @@ static int ctrl_cmd_delete(const char *input)
 	/* Counter sets are only on the uid tag, not full tag */
 	tcs_entry = tag_counter_set_tree_search(&tag_counter_set_tree, tag);
 	if (tcs_entry) {
-		CT_DEBUG("qtaguid: ctrl_delete(): "
+		CT_DEBUG("qtaguid: ctrl_delete(%s): "
 			 "erase tcs: tag=0x%llx (uid=%u) set=%d\n",
+			 input,
 			 tcs_entry->tn.tag,
 			 get_uid_from_tag(tcs_entry->tn.tag),
 			 tcs_entry->active_set);
@@ -1891,16 +1943,16 @@ static int ctrl_cmd_delete(const char *input)
 			entry_uid = get_uid_from_tag(ts_entry->tn.tag);
 			node = rb_next(node);
 
-			CT_DEBUG("qtaguid: ctrl_delete(): "
+			CT_DEBUG("qtaguid: ctrl_delete(%s): "
 				 "ts tag=0x%llx (uid=%u)\n",
-				 ts_entry->tn.tag, entry_uid);
+				 input, ts_entry->tn.tag, entry_uid);
 
 			if (entry_uid != uid)
 				continue;
 			if (!acct_tag || ts_entry->tn.tag == tag) {
-				CT_DEBUG("qtaguid: ctrl_delete(): "
+				CT_DEBUG("qtaguid: ctrl_delete(%s): "
 					 "erase ts: %s 0x%llx %u\n",
-					 iface_entry->ifname,
+					 input, iface_entry->ifname,
 					 get_atag_from_tag(ts_entry->tn.tag),
 					 entry_uid);
 				rb_erase(&ts_entry->tn.node,
@@ -1920,9 +1972,9 @@ static int ctrl_cmd_delete(const char *input)
 		entry_uid = utd_entry->uid;
 		node = rb_next(node);
 
-		CT_DEBUG("qtaguid: ctrl_delete(): "
+		CT_DEBUG("qtaguid: ctrl_delete(%s): "
 			 "utd uid=%u\n",
-			 entry_uid);
+			 input, entry_uid);
 
 		if (entry_uid != uid)
 			continue;
@@ -2377,7 +2429,7 @@ static int pp_stats_line(struct proc_print_info *ppi, int cnt_set)
 	return len;
 }
 
-bool pp_sets(struct proc_print_info *ppi)
+static bool pp_sets(struct proc_print_info *ppi)
 {
 	int len;
 	int counter_set;
@@ -2472,7 +2524,7 @@ static int qtudev_open(struct inode *inode, struct file *file)
 {
 	struct uid_tag_data *utd_entry;
 	struct proc_qtu_data  *pqd_entry;
-	struct proc_qtu_data  *new_pqd_entry = 0;
+	struct proc_qtu_data  *new_pqd_entry;
 	int res;
 	bool utd_entry_found;
 
@@ -2514,12 +2566,14 @@ static int qtudev_open(struct inode *inode, struct file *file)
 	new_pqd_entry->pid = current->tgid;
 	INIT_LIST_HEAD(&new_pqd_entry->sock_tag_list);
 	new_pqd_entry->parent_tag_data = utd_entry;
+	utd_entry->num_pqd++;
 
 	proc_qtu_data_tree_insert(new_pqd_entry,
 				  &proc_qtu_data_tree);
 
 	spin_unlock_bh(&uid_tag_data_tree_lock);
-	DR_DEBUG("qtaguid: tracking data for uid=%u\n", current_fsuid());
+	DR_DEBUG("qtaguid: tracking data for uid=%u in pqd=%p\n",
+		 current_fsuid(), new_pqd_entry);
 	file->private_data = new_pqd_entry;
 	return 0;
 
@@ -2559,12 +2613,6 @@ static int qtudev_release(struct inode *inode, struct file *file)
 	spin_lock_bh(&sock_tag_list_lock);
 	spin_lock_bh(&uid_tag_data_tree_lock);
 
-	/*
-	 * If this proc didn't actually tag anything for itself, or has already
-	 * willingly cleaned up itself ...
-	 */
-	put_utd_entry(utd_entry);
-
 	list_for_each_safe(entry, next, &pqd_entry->sock_tag_list) {
 		st_entry = list_entry(entry, struct sock_tag, list);
 		DR_DEBUG("qtaguid: %s(): "
@@ -2593,10 +2641,18 @@ static int qtudev_release(struct inode *inode, struct file *file)
 		/* Can't sockfd_put() within spinlock, do it later. */
 		sock_tag_tree_insert(st_entry, &st_to_free_tree);
 
-		/* Do not put_utd_entry(utd_entry) someone elses utd_entry */
+		/*
+		 * Try to free the utd_entry if no other proc_qtu_data is
+		 * using it (num_pqd is 0) and it doesn't have active tags
+		 * (num_active_tags is 0).
+		 */
+		put_utd_entry(utd_entry);
 	}
 
 	rb_erase(&pqd_entry->node, &proc_qtu_data_tree);
+	BUG_ON(pqd_entry->parent_tag_data->num_pqd < 1);
+	pqd_entry->parent_tag_data->num_pqd--;
+	put_utd_entry(pqd_entry->parent_tag_data);
 	kfree(pqd_entry);
 	file->private_data = NULL;
 
@@ -2606,7 +2662,8 @@ 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__,
+			   current->pid, current->tgid);
 	return 0;
 }
 
diff --git a/net/netfilter/xt_qtaguid_internal.h b/net/netfilter/xt_qtaguid_internal.h
index d39ee89a9a2c..fdce0d006d30 100644
--- a/net/netfilter/xt_qtaguid_internal.h
+++ b/net/netfilter/xt_qtaguid_internal.h
@@ -15,31 +15,33 @@
 #include <linux/spinlock_types.h>
 #include <linux/workqueue.h>
 
-/* Define/comment out these *DEBUG to compile in/out the pr_debug calls. */
 /* Iface handling */
-#define IDEBUG
+#define IDEBUG_MASK (1<<0)
 /* Iptable Matching. Per packet. */
-#define MDEBUG
+#define MDEBUG_MASK (1<<1)
 /* Red-black tree handling. Per packet. */
-#define RDEBUG
+#define RDEBUG_MASK (1<<2)
 /* procfs ctrl/stats handling */
-#define CDEBUG
+#define CDEBUG_MASK (1<<3)
 /* dev and resource tracking */
-#define DDEBUG
+#define DDEBUG_MASK (1<<4)
 
 /* E.g (IDEBUG_MASK | CDEBUG_MASK | DDEBUG_MASK) */
 #define DEFAULT_DEBUG_MASK 0
 
+/*
+ * (Un)Define these *DEBUG to compile out/in the pr_debug calls.
+ * All undef: text size ~ 0x3030; all def: ~ 0x4404.
+ */
+#define IDEBUG
+#define MDEBUG
+#define RDEBUG
+#define CDEBUG
+#define DDEBUG
 
-#define IDEBUG_MASK (1<<0)
-#define MDEBUG_MASK (1<<1)
-#define RDEBUG_MASK (1<<2)
-#define CDEBUG_MASK (1<<3)
-#define DDEBUG_MASK (1<<4)
-
-#define MSK_DEBUG(mask, ...) do {                       \
-		if (unlikely(debug_mask & (mask)))      \
-			pr_debug(__VA_ARGS__);          \
+#define MSK_DEBUG(mask, ...) do {                           \
+		if (unlikely(qtaguid_debug_mask & (mask)))  \
+			pr_debug(__VA_ARGS__);              \
 	} while (0)
 #ifdef IDEBUG
 #define IF_DEBUG(...) MSK_DEBUG(IDEBUG_MASK, __VA_ARGS__)
@@ -67,7 +69,7 @@
 #define DR_DEBUG(...) no_printk(__VA_ARGS__)
 #endif
 
-extern uint debug_mask;
+extern uint qtaguid_debug_mask;
 
 /*---------------------------------------------------------------------------*/
 /*
@@ -286,6 +288,8 @@ struct uid_tag_data {
 	 * For the uid, how many accounting tags have been set.
 	 */
 	int num_active_tags;
+	/* Track the number of proc_qtu_data that reference it */
+	int num_pqd;
 	struct rb_root tag_ref_tree;
 	/* No tag_node_tree_lock; use uid_tag_data_tree_lock */
 };
diff --git a/net/netfilter/xt_qtaguid_print.c b/net/netfilter/xt_qtaguid_print.c
index 45d717ff3653..39176785c91f 100644
--- a/net/netfilter/xt_qtaguid_print.c
+++ b/net/netfilter/xt_qtaguid_print.c
@@ -9,9 +9,13 @@
  */
 
 /*
- * There are run-time debug flags enabled via the debug_mask module param, or
- * via the DEFAULT_DEBUG_MASK. See xt_qtaguid_internal.h.
+ * Most of the functions in this file just waste time if DEBUG is not defined.
+ * The matching xt_qtaguid_print.h will static inline empty funcs if the needed
+ * debug flags ore not defined.
+ * Those funcs that fail to allocate memory will panic as there is no need to
+ * hobble allong just pretending to do the requested work.
  */
+
 #define DEBUG
 
 #include <linux/fs.h>
@@ -25,21 +29,38 @@
 #include "xt_qtaguid_internal.h"
 #include "xt_qtaguid_print.h"
 
+#ifdef DDEBUG
+
+static void _bug_on_err_or_null(void *ptr)
+{
+	if (IS_ERR_OR_NULL(ptr)) {
+		pr_err("qtaguid: kmalloc failed\n");
+		BUG();
+	}
+}
+
 char *pp_tag_t(tag_t *tag)
 {
+	char *res;
+
 	if (!tag)
-		return kasprintf(GFP_ATOMIC, "tag_t@null{}");
-	return kasprintf(GFP_ATOMIC,
-			 "tag_t@%p{tag=0x%llx, uid=%u}",
-			 tag, *tag, get_uid_from_tag(*tag));
+		res = kasprintf(GFP_ATOMIC, "tag_t@null{}");
+	else
+		res = kasprintf(GFP_ATOMIC,
+				"tag_t@%p{tag=0x%llx, uid=%u}",
+				tag, *tag, get_uid_from_tag(*tag));
+	_bug_on_err_or_null(res);
+	return res;
 }
 
 char *pp_data_counters(struct data_counters *dc, bool showValues)
 {
+	char *res;
+
 	if (!dc)
-		return kasprintf(GFP_ATOMIC, "data_counters@null{}");
-	if (showValues)
-		return kasprintf(
+		res = kasprintf(GFP_ATOMIC, "data_counters@null{}");
+	else if (showValues)
+		res = kasprintf(
 			GFP_ATOMIC, "data_counters@%p{"
 			"set0{"
 			"rx{"
@@ -85,7 +106,9 @@ char *pp_data_counters(struct data_counters *dc, bool showValues)
 			dc->bpc[1][IFS_TX][IFS_PROTO_OTHER].bytes,
 			dc->bpc[1][IFS_TX][IFS_PROTO_OTHER].packets);
 	else
-		return kasprintf(GFP_ATOMIC, "data_counters@%p{...}", dc);
+		res = kasprintf(GFP_ATOMIC, "data_counters@%p{...}", dc);
+	_bug_on_err_or_null(res);
+	return res;
 }
 
 char *pp_tag_node(struct tag_node *tn)
@@ -93,12 +116,16 @@ char *pp_tag_node(struct tag_node *tn)
 	char *tag_str;
 	char *res;
 
-	if (!tn)
-		return kasprintf(GFP_ATOMIC, "tag_node@null{}");
+	if (!tn) {
+		res = kasprintf(GFP_ATOMIC, "tag_node@null{}");
+		_bug_on_err_or_null(res);
+		return res;
+	}
 	tag_str = pp_tag_t(&tn->tag);
 	res = kasprintf(GFP_ATOMIC,
 			"tag_node@%p{tag=%s}",
 			tn, tag_str);
+	_bug_on_err_or_null(res);
 	kfree(tag_str);
 	return res;
 }
@@ -108,12 +135,16 @@ char *pp_tag_ref(struct tag_ref *tr)
 	char *tn_str;
 	char *res;
 
-	if (!tr)
-		return kasprintf(GFP_ATOMIC, "tag_ref@null{}");
+	if (!tr) {
+		res = kasprintf(GFP_ATOMIC, "tag_ref@null{}");
+		_bug_on_err_or_null(res);
+		return res;
+	}
 	tn_str = pp_tag_node(&tr->tn);
 	res = kasprintf(GFP_ATOMIC,
 			"tag_ref@%p{%s, num_sock_tags=%d}",
 			tr, tn_str, tr->num_sock_tags);
+	_bug_on_err_or_null(res);
 	kfree(tn_str);
 	return res;
 }
@@ -125,14 +156,18 @@ char *pp_tag_stat(struct tag_stat *ts)
 	char *parent_counters_str;
 	char *res;
 
-	if (!ts)
-		return kasprintf(GFP_ATOMIC, "tag_stat@null{}");
+	if (!ts) {
+		res = kasprintf(GFP_ATOMIC, "tag_stat@null{}");
+		_bug_on_err_or_null(res);
+		return res;
+	}
 	tn_str = pp_tag_node(&ts->tn);
 	counters_str = pp_data_counters(&ts->counters, true);
 	parent_counters_str = pp_data_counters(ts->parent_counters, false);
 	res = kasprintf(GFP_ATOMIC,
 			"tag_stat@%p{%s, counters=%s, parent_counters=%s}",
 			ts, tn_str, counters_str, parent_counters_str);
+	_bug_on_err_or_null(res);
 	kfree(tn_str);
 	kfree(counters_str);
 	kfree(parent_counters_str);
@@ -141,38 +176,42 @@ char *pp_tag_stat(struct tag_stat *ts)
 
 char *pp_iface_stat(struct iface_stat *is)
 {
+	char *res;
 	if (!is)
-		return kasprintf(GFP_ATOMIC, "iface_stat@null{}");
-	return kasprintf(GFP_ATOMIC, "iface_stat@%p{"
-			 "list=list_head{...}, "
-			 "ifname=%s, "
-			 "total={rx={bytes=%llu, "
-			 "packets=%llu}, "
-			 "tx={bytes=%llu, "
-			 "packets=%llu}}, "
-			 "last_known_valid=%d, "
-			 "last_known={rx={bytes=%llu, "
-			 "packets=%llu}, "
-			 "tx={bytes=%llu, "
-			 "packets=%llu}}, "
-			 "active=%d, "
-			 "net_dev=%p, "
-			 "proc_ptr=%p, "
-			 "tag_stat_tree=rb_root{...}}",
-			 is,
-			 is->ifname,
-			 is->totals[IFS_RX].bytes,
-			 is->totals[IFS_RX].packets,
-			 is->totals[IFS_TX].bytes,
-			 is->totals[IFS_TX].packets,
-			 is->last_known_valid,
-			 is->last_known[IFS_RX].bytes,
-			 is->last_known[IFS_RX].packets,
-			 is->last_known[IFS_TX].bytes,
-			 is->last_known[IFS_TX].packets,
-			 is->active,
-			 is->net_dev,
-			 is->proc_ptr);
+		res = kasprintf(GFP_ATOMIC, "iface_stat@null{}");
+	else
+		res = kasprintf(GFP_ATOMIC, "iface_stat@%p{"
+				"list=list_head{...}, "
+				"ifname=%s, "
+				"total={rx={bytes=%llu, "
+				"packets=%llu}, "
+				"tx={bytes=%llu, "
+				"packets=%llu}}, "
+				"last_known_valid=%d, "
+				"last_known={rx={bytes=%llu, "
+				"packets=%llu}, "
+				"tx={bytes=%llu, "
+				"packets=%llu}}, "
+				"active=%d, "
+				"net_dev=%p, "
+				"proc_ptr=%p, "
+				"tag_stat_tree=rb_root{...}}",
+				is,
+				is->ifname,
+				is->totals[IFS_RX].bytes,
+				is->totals[IFS_RX].packets,
+				is->totals[IFS_TX].bytes,
+				is->totals[IFS_TX].packets,
+				is->last_known_valid,
+				is->last_known[IFS_RX].bytes,
+				is->last_known[IFS_RX].packets,
+				is->last_known[IFS_TX].bytes,
+				is->last_known[IFS_TX].packets,
+				is->active,
+				is->net_dev,
+				is->proc_ptr);
+	_bug_on_err_or_null(res);
+	return res;
 }
 
 char *pp_sock_tag(struct sock_tag *st)
@@ -180,8 +219,11 @@ char *pp_sock_tag(struct sock_tag *st)
 	char *tag_str;
 	char *res;
 
-	if (!st)
-		return kasprintf(GFP_ATOMIC, "sock_tag@null{}");
+	if (!st) {
+		res = kasprintf(GFP_ATOMIC, "sock_tag@null{}");
+		_bug_on_err_or_null(res);
+		return res;
+	}
 	tag_str = pp_tag_t(&st->tag);
 	res = kasprintf(GFP_ATOMIC, "sock_tag@%p{"
 			"sock_node=rb_node{...}, "
@@ -190,6 +232,7 @@ char *pp_sock_tag(struct sock_tag *st)
 			st, st->sk, st->socket, atomic_long_read(
 				&st->socket->file->f_count),
 			st->pid, tag_str);
+	_bug_on_err_or_null(res);
 	kfree(tag_str);
 	return res;
 }
@@ -199,13 +242,16 @@ char *pp_uid_tag_data(struct uid_tag_data *utd)
 	char *res;
 
 	if (!utd)
-		return kasprintf(GFP_ATOMIC, "uid_tag_data@null{}");
-	res = kasprintf(GFP_ATOMIC, "uid_tag_data@%p{"
-			"uid=%u, num_active_acct_tags=%d, "
-			"tag_node_tree=rb_root{...}, "
-			"proc_qtu_data_tree=rb_root{...}}",
-			utd, utd->uid,
-			utd->num_active_tags);
+		res = kasprintf(GFP_ATOMIC, "uid_tag_data@null{}");
+	else
+		res = kasprintf(GFP_ATOMIC, "uid_tag_data@%p{"
+				"uid=%u, num_active_acct_tags=%d, "
+				"num_pqd=%d, "
+				"tag_node_tree=rb_root{...}, "
+				"proc_qtu_data_tree=rb_root{...}}",
+				utd, utd->uid,
+				utd->num_active_tags, utd->num_pqd);
+	_bug_on_err_or_null(res);
 	return res;
 }
 
@@ -214,8 +260,11 @@ char *pp_proc_qtu_data(struct proc_qtu_data *pqd)
 	char *parent_tag_data_str;
 	char *res;
 
-	if (!pqd)
-		return kasprintf(GFP_ATOMIC, "proc_qtu_data@null{}");
+	if (!pqd) {
+		res = kasprintf(GFP_ATOMIC, "proc_qtu_data@null{}");
+		_bug_on_err_or_null(res);
+		return res;
+	}
 	parent_tag_data_str = pp_uid_tag_data(pqd->parent_tag_data);
 	res = kasprintf(GFP_ATOMIC, "proc_qtu_data@%p{"
 			"node=rb_node{...}, pid=%u, "
@@ -223,6 +272,7 @@ char *pp_proc_qtu_data(struct proc_qtu_data *pqd)
 			"sock_tag_list=list_head{...}}",
 			pqd, pqd->pid, parent_tag_data_str
 		);
+	_bug_on_err_or_null(res);
 	kfree(parent_tag_data_str);
 	return res;
 }
@@ -235,20 +285,29 @@ void prdebug_sock_tag_tree(int indent_level,
 	struct sock_tag *sock_tag_entry;
 	char *str;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (RB_EMPTY_ROOT(sock_tag_tree)) {
+		str = "sock_tag_tree=rb_root{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "sock_tag_tree=rb_root{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	for (node = rb_first(sock_tag_tree);
 	     node;
 	     node = rb_next(node)) {
 		sock_tag_entry = rb_entry(node, struct sock_tag, sock_node);
 		str = pp_sock_tag(sock_tag_entry);
-		CT_DEBUG("%*d: %s,\n", indent_level*2, indent_level, str);
+		pr_debug("%*d: %s,\n", indent_level*2, indent_level, str);
 		kfree(str);
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
 void prdebug_sock_tag_list(int indent_level,
@@ -257,17 +316,26 @@ void prdebug_sock_tag_list(int indent_level,
 	struct sock_tag *sock_tag_entry;
 	char *str;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (list_empty(sock_tag_list)) {
+		str = "sock_tag_list=list_head{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "sock_tag_list=list_head{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	list_for_each_entry(sock_tag_entry, sock_tag_list, list) {
 		str = pp_sock_tag(sock_tag_entry);
-		CT_DEBUG("%*d: %s,\n", indent_level*2, indent_level, str);
+		pr_debug("%*d: %s,\n", indent_level*2, indent_level, str);
 		kfree(str);
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
 void prdebug_proc_qtu_data_tree(int indent_level,
@@ -277,8 +345,17 @@ void prdebug_proc_qtu_data_tree(int indent_level,
 	struct rb_node *node;
 	struct proc_qtu_data *proc_qtu_data_entry;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (RB_EMPTY_ROOT(proc_qtu_data_tree)) {
+		str = "proc_qtu_data_tree=rb_root{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "proc_qtu_data_tree=rb_root{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	for (node = rb_first(proc_qtu_data_tree);
 	     node;
@@ -287,7 +364,7 @@ void prdebug_proc_qtu_data_tree(int indent_level,
 					       struct proc_qtu_data,
 					       node);
 		str = pp_proc_qtu_data(proc_qtu_data_entry);
-		CT_DEBUG("%*d: %s,\n", indent_level*2, indent_level,
+		pr_debug("%*d: %s,\n", indent_level*2, indent_level,
 			 str);
 		kfree(str);
 		indent_level++;
@@ -298,7 +375,7 @@ void prdebug_proc_qtu_data_tree(int indent_level,
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
 void prdebug_tag_ref_tree(int indent_level, struct rb_root *tag_ref_tree)
@@ -307,8 +384,17 @@ void prdebug_tag_ref_tree(int indent_level, struct rb_root *tag_ref_tree)
 	struct rb_node *node;
 	struct tag_ref *tag_ref_entry;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (RB_EMPTY_ROOT(tag_ref_tree)) {
+		str = "tag_ref_tree{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "tag_ref_tree{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	for (node = rb_first(tag_ref_tree);
 	     node;
@@ -317,13 +403,13 @@ void prdebug_tag_ref_tree(int indent_level, struct rb_root *tag_ref_tree)
 					 struct tag_ref,
 					 tn.node);
 		str = pp_tag_ref(tag_ref_entry);
-		CT_DEBUG("%*d: %s,\n", indent_level*2, indent_level,
+		pr_debug("%*d: %s,\n", indent_level*2, indent_level,
 			 str);
 		kfree(str);
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
 void prdebug_uid_tag_data_tree(int indent_level,
@@ -333,8 +419,17 @@ void prdebug_uid_tag_data_tree(int indent_level,
 	struct rb_node *node;
 	struct uid_tag_data *uid_tag_data_entry;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (RB_EMPTY_ROOT(uid_tag_data_tree)) {
+		str = "uid_tag_data_tree=rb_root{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "uid_tag_data_tree=rb_root{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	for (node = rb_first(uid_tag_data_tree);
 	     node;
@@ -342,7 +437,7 @@ void prdebug_uid_tag_data_tree(int indent_level,
 		uid_tag_data_entry = rb_entry(node, struct uid_tag_data,
 					      node);
 		str = pp_uid_tag_data(uid_tag_data_entry);
-		CT_DEBUG("%*d: %s,\n", indent_level*2, indent_level, str);
+		pr_debug("%*d: %s,\n", indent_level*2, indent_level, str);
 		kfree(str);
 		if (!RB_EMPTY_ROOT(&uid_tag_data_entry->tag_ref_tree)) {
 			indent_level++;
@@ -353,7 +448,7 @@ void prdebug_uid_tag_data_tree(int indent_level,
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
 void prdebug_tag_stat_tree(int indent_level,
@@ -363,21 +458,30 @@ void prdebug_tag_stat_tree(int indent_level,
 	struct rb_node *node;
 	struct tag_stat *ts_entry;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (RB_EMPTY_ROOT(tag_stat_tree)) {
+		str = "tag_stat_tree{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "tag_stat_tree{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	for (node = rb_first(tag_stat_tree);
 	     node;
 	     node = rb_next(node)) {
 		ts_entry = rb_entry(node, struct tag_stat, tn.node);
 		str = pp_tag_stat(ts_entry);
-		CT_DEBUG("%*d: %s\n", indent_level*2, indent_level,
+		pr_debug("%*d: %s\n", indent_level*2, indent_level,
 			 str);
 		kfree(str);
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
 void prdebug_iface_stat_list(int indent_level,
@@ -386,12 +490,21 @@ void prdebug_iface_stat_list(int indent_level,
 	char *str;
 	struct iface_stat *iface_entry;
 
+	if (!unlikely(qtaguid_debug_mask & DDEBUG_MASK))
+		return;
+
+	if (list_empty(iface_stat_list)) {
+		str = "iface_stat_list=list_head{}";
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
+		return;
+	}
+
 	str = "iface_stat_list=list_head{";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 	indent_level++;
 	list_for_each_entry(iface_entry, iface_stat_list, list) {
 		str = pp_iface_stat(iface_entry);
-		CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+		pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 		kfree(str);
 
 		spin_lock_bh(&iface_entry->tag_stat_list_lock);
@@ -405,9 +518,10 @@ void prdebug_iface_stat_list(int indent_level,
 	}
 	indent_level--;
 	str = "}";
-	CT_DEBUG("%*d: %s\n", indent_level*2, indent_level, str);
+	pr_debug("%*d: %s\n", indent_level*2, indent_level, str);
 }
 
+#endif  /* ifdef DDEBUG */
 /*------------------------------------------*/
 static const char * const netdev_event_strings[] = {
 	"netdev_unknown",
diff --git a/net/netfilter/xt_qtaguid_print.h b/net/netfilter/xt_qtaguid_print.h
index 388622860ea0..b63871a0be5a 100644
--- a/net/netfilter/xt_qtaguid_print.h
+++ b/net/netfilter/xt_qtaguid_print.h
@@ -12,6 +12,8 @@
 
 #include "xt_qtaguid_internal.h"
 
+#ifdef DDEBUG
+
 char *pp_tag_t(tag_t *tag);
 char *pp_data_counters(struct data_counters *dc, bool showValues);
 char *pp_tag_node(struct tag_node *tn);
@@ -37,6 +39,82 @@ void prdebug_tag_stat_tree(int indent_level,
 void prdebug_iface_stat_list(int indent_level,
 			     struct list_head *iface_stat_list);
 
+#else
+
+/*------------------------------------------*/
+static inline char *pp_tag_t(tag_t *tag)
+{
+	return NULL;
+}
+static inline char *pp_data_counters(struct data_counters *dc, bool showValues)
+{
+	return NULL;
+}
+static inline char *pp_tag_node(struct tag_node *tn)
+{
+	return NULL;
+}
+static inline char *pp_tag_ref(struct tag_ref *tr)
+{
+	return NULL;
+}
+static inline char *pp_tag_stat(struct tag_stat *ts)
+{
+	return NULL;
+}
+static inline char *pp_iface_stat(struct iface_stat *is)
+{
+	return NULL;
+}
+static inline char *pp_sock_tag(struct sock_tag *st)
+{
+	return NULL;
+}
+static inline char *pp_uid_tag_data(struct uid_tag_data *qtd)
+{
+	return NULL;
+}
+static inline char *pp_proc_qtu_data(struct proc_qtu_data *pqd)
+{
+	return NULL;
+}
+
+/*------------------------------------------*/
+static inline
+void prdebug_sock_tag_list(int indent_level,
+			   struct list_head *sock_tag_list)
+{
+}
+static inline
+void prdebug_sock_tag_tree(int indent_level,
+			   struct rb_root *sock_tag_tree)
+{
+}
+static inline
+void prdebug_proc_qtu_data_tree(int indent_level,
+				struct rb_root *proc_qtu_data_tree)
+{
+}
+static inline
+void prdebug_tag_ref_tree(int indent_level, struct rb_root *tag_ref_tree)
+{
+}
+static inline
+void prdebug_uid_tag_data_tree(int indent_level,
+			       struct rb_root *uid_tag_data_tree)
+{
+}
+static inline
+void prdebug_tag_stat_tree(int indent_level,
+			   struct rb_root *tag_stat_tree)
+{
+}
+static inline
+void prdebug_iface_stat_list(int indent_level,
+			     struct list_head *iface_stat_list)
+{
+}
+#endif
 /*------------------------------------------*/
 const char *netdev_evt_str(int netdev_event);
 #endif  /* ifndef __XT_QTAGUID_PRINT_H__ */