Merge pull request #35 from OpenCloudOS/dev

some bug fix

Author: menglongdong

component/sys_utils.c

@@ -33,19 +33,16 @@ int exec(char *cmd, char *output)
 		strcat(output + strlen(output), buf);
 	}
 
-	pr_debug("command: %s\n", cmd);
 	status = pclose(f);
+	pr_debug("command: %s, status:%d\n", cmd, WEXITSTATUS(status));
 	return WEXITSTATUS(status);
 }
 
 int execf(char *output, char *fmt, ...)
 {
-	char *cmd = malloc(1024);
+	static char cmd[1024];
 	va_list valist;
 
-	if (!cmd)
-		return -ENOMEM;
-
 	va_start(valist, fmt);
 	vsprintf(cmd, fmt, valist);
 	va_end(valist);

src/gen_trace.py

@@ -23,6 +23,40 @@
 }
 
 
+def parse_names(trace, children):
+    names = trace['names']
+    first = names[0]
+    prev = None
+    if isinstance(first, str):
+        trace['name'] = first
+    else:
+        trace['name'] = first['name']
+        trace['cond'] = first['cond'].replace('"', '\\"')
+
+    del trace['names']
+    prev = trace
+
+    for index in range(1, len(names)):
+        name = names[index]
+        new_child = dict(trace)
+
+        if isinstance(name, str):
+            new_child['name'] = name
+            del new_child['cond']
+        else:
+            new_child['name'] = name['name']
+            if 'cond' in name:
+                new_child['cond'] = name['cond'].replace('"', '\\"')
+            else:
+                del new_child['cond']
+
+        prev['next'] = new_child
+        prev = new_child
+        children.append(new_child)
+
+    prev['next'] = trace
+
+
 def parse_group(group):
     """ parse group in yaml file """
     if 'children' not in group:
@@ -34,13 +68,8 @@ def parse_group(group):
         if not isinstance(child, str):
             parse_group(child)
             i += 1
-            names = child['name']
-            if isinstance(names, list):
-                child['name'] = names[0]
-                for name in names[1:]:
-                    new_child = dict(child)
-                    new_child['name'] = name
-                    children.append(new_child)
+            if 'names' in child:
+                parse_names(child, children)
             name_split = child['name'].split(':')
             if len(name_split) > 1:
                 child['name'] = name_split[0]
@@ -146,7 +175,7 @@ def gen_trace(trace, group, p_name):
         (rule_str, _init_str) = gen_rules(rules, trace_name)
         init_str += _init_str
 
-    if_str = f'\n\t.if_str = "{trace.get("if")}",' if 'if' in trace else ''
+    cond_str = f'\n\t.cond = "{trace["cond"]}",' if 'cond' in trace else ''
     reg_str = f'\n\t.regex = "{trace["regex"]}",' if 'regex' in trace else ''
     msg_str = f'\n\t.msg = "{trace["msg"]}",' if 'msg' in trace else ''
     default = True
@@ -159,18 +188,19 @@ def gen_trace(trace, group, p_name):
     target = trace.get('target') or trace['name']
     define_str = f'''trace_t {trace_name} = {{
 \t.name = "{target}",
-\t.desc = "{trace.get('desc')}",{skb_str}{if_str}{default}
+\t.desc = "{trace.get('desc') or ''}",{skb_str}{cond_str}{default}
 \t.type = {trace_type},{reg_str}{analyzer}{msg_str}
 \t.index = INDEX_{name},
 \t.prog = "__trace_{name}",
 \t.parent = &{p_name},
 \t.rules =  LIST_HEAD_INIT({trace_name}.rules),
+\t.mutex = {'true' if trace.get('mutex') else 'false'},
 }};
 {rule_str}
 '''
     init_str += f'''\tlist_add_tail(&{trace_name}.list, &{p_name}.traces);
 \tall_traces[INDEX_{name}] = &{trace_name};
-\tlist_add_tail(&{trace_name}.sibling, &trace_list);
+\tlist_add_tail(&{trace_name}.all, &trace_list);
 '''
     global_status['trace_index'] += 1
 
@@ -217,6 +247,13 @@ def gen_group(group, is_root=False):
         else:
             gen_append(result, gen_trace(child, group, p_name))
 
+    for child in group['children']:
+        if 'children' in child:
+            continue
+        sibling = 'NULL'
+        if 'next' in child:
+            sibling = f"&trace_{child['next']['define_name']}"
+        result['init_str'] += f"\ttrace_{child['define_name']}.sibling = {sibling};\n"
     return result
 
 

src/progs/kprobe.c

@@ -190,7 +190,7 @@ static try_inline int handle_exit(struct pt_regs *regs, int func)
 }
 
 /* one trace may have more than one implement */
-#define __DEFINE_KPROBE_RAW(name, func_name, skb_init)		\
+#define __DEFINE_KPROBE_INIT(name, func_name, skb_init)		\
 	static try_inline int fake__##name(struct pt_regs *ctx,	\
 				       struct sk_buff *skb,	\
 				       int func);		\
@@ -209,11 +209,15 @@ static try_inline int handle_exit(struct pt_regs *regs, int func)
 				       struct sk_buff *skb,	\
 				       int func)
 
-#define DEFINE_KPROBE_RAW(name, skb_init)			\
-	__DEFINE_KPROBE_RAW(name, name, skb_init)
+#define DEFINE_KPROBE_INIT(name, skb_init)			\
+	__DEFINE_KPROBE_INIT(name, name, skb_init)
 
 #define DEFINE_KPROBE(name, skb_index)				\
-	DEFINE_KPROBE_RAW(name, PT_REGS_PARM##skb_index(ctx))
+	DEFINE_KPROBE_INIT(name, PT_REGS_PARM##skb_index(ctx))
+
+#define DEFINE_KPROBE_TARGET(name, func_name, skb_index)	\
+	__DEFINE_KPROBE_INIT(name, func_name,			\
+			    PT_REGS_PARM##skb_index(ctx))
 
 #define KPROBE_DEFAULT(name, skb_index)				\
 	DEFINE_KPROBE(name, skb_index)				\
@@ -262,23 +266,37 @@ DEFINE_TP(kfree_skb, skb, kfree_skb, 8)
 	return 0;
 }
 
-__DEFINE_KPROBE_RAW(__netif_receive_skb_core_pskb, __netif_receive_skb_core,
-		  _(*(void **)(PT_REGS_PARM1(ctx))))
+__DEFINE_KPROBE_INIT(__netif_receive_skb_core_pskb, __netif_receive_skb_core,
+		    _(*(void **)(PT_REGS_PARM1(ctx))))
 {
 	return default_handle_entry(ctx, skb, func);
 }
 
+#define bpf_ipt_do_table()						\
+{									\
+	nf_event_t e = {						\
+		.event = { .func = func, },				\
+		.hook = _C(state, hook),				\
+	};								\
+									\
+	bpf_probe_read(e.table, sizeof(e.table) - 1, _C(table, name));	\
+	return handle_entry(ctx, skb, &e.event, sizeof(e), func);	\
+}
+
 DEFINE_KPROBE(ipt_do_table, 1)
 {
 	struct nf_hook_state *state = (void *)PT_REGS_PARM2(ctx);
 	struct xt_table *table = (void *)PT_REGS_PARM3(ctx);
-	nf_event_t e = {
-		.event = { .func = func, },
-		.hook = _C(state, hook),
-	};
 
-	bpf_probe_read(e.table, sizeof(e.table) - 1, _C(table, name));
-	return handle_entry(ctx, skb, &e.event, sizeof(e), func);
+	bpf_ipt_do_table();
+}
+
+DEFINE_KPROBE_TARGET(ipt_do_table_new, ipt_do_table, 2)
+{
+	struct nf_hook_state *state = (void *)PT_REGS_PARM3(ctx);
+	struct xt_table *table = (void *)PT_REGS_PARM1(ctx);
+
+	bpf_ipt_do_table();
 }
 
 DEFINE_KPROBE(nf_hook_slow, 1)

src/progs/nft_do_chain.c

@@ -81,14 +81,14 @@ static try_inline int BPF_NAME(struct pt_regs *ctx, int func)
 }
 
 /* another magical macro definiation */
-#define ___DEFINE_KPROBE_RAW(name, target)	\
-	__DEFINE_KPROBE_RAW(name, target, NULL)
+#define ___DEFINE_KPROBE_INIT(name, target)	\
+	__DEFINE_KPROBE_INIT(name, target, NULL)
 
 /**
  * This function is used to the kernel version that don't support
  * kernel module BTF.
  */
-___DEFINE_KPROBE_RAW(NFT_NAME, nft_do_chain)
+___DEFINE_KPROBE_INIT(NFT_NAME, nft_do_chain)
 {
 	return BPF_NAME(ctx, func);
 }

src/trace.c

@@ -10,6 +10,10 @@
 #include "analysis.h"
 #include "dropreason.h"
 
+const char *cond_pre = "verlte() { [ \"$1\" = \"$2\" ] && echo 0 && return; "
+		       "[ \"$1\" = \"$(/bin/echo -e \"$1\\n$2\" | sort -V | head -n1)\" ] "
+		       "&& echo -1 && return; echo 1; }";
+
 trace_context_t trace_ctx = {
 	.mode = TRACE_MODE_TIMELINE,
 };
@@ -272,40 +276,52 @@ static int trace_prepare_args()
 	return -1;
 }
 
+static void trace_exec_cond()
+{
+	trace_t *trace, *sibling;
+	bool mutexed;
+
+	trace_for_each(trace) {
+		if (trace->mutex && (sibling = trace->sibling)) {
+			mutexed = false;
+			while (sibling != trace) {
+				if ((sibling->status & TRACE_CHECKED) &&
+				    !trace_is_invalid(sibling)) {
+					mutexed = true;
+					break;
+				}
+				sibling = sibling->sibling;
+			}
+			if (mutexed) {
+				trace_set_invalid(trace);
+				pr_debug("mutex happens in %s\n", trace->name);
+				goto next;
+			}
+		}
+
+		if (trace->cond) {
+			if (execf(NULL, "%s; %s", cond_pre, trace->cond))
+				trace_set_invalid(trace);
+		}
+next:
+		trace->status |= TRACE_CHECKED;
+	}
+}
+
 static int trace_prepare_traces()
 {
 	char func[128], name[136], *fmt;
 	trace_t *trace;
 	int sym_type;
 
-	/* high version kernel (>=5.4) use (struct sk_buff**) */
-	if (kv_compare(5, 4, 0) < 0)
-		trace_set_invalid(&trace___netif_receive_skb_core_pskb);
-	else
-		trace_set_invalid(&trace___netif_receive_skb_core);
+	trace_exec_cond();
 
 	/* from v5.14, the struct of nft_pktinfo changed */
 	if (kv_compare(5, 14, 0) >= 0) {
 		trace_ctx.bpf_args.nft_high = true;
 		pr_debug("nft high version founded\n");
 	}
 
-	sym_type = sym_get_type("nft_do_chain");
-	if (sym_type == SYM_NOT_EXIST) {
-		trace_set_invalid(&trace_nft_do_chain_compat);
-		trace_set_invalid(&trace_nft_do_chain);
-	} else if (!kernel_has_config("DEBUG_INFO_BTF_MODULES") &&
-		   sym_type == SYM_MODULE) {
-
-		/* BTF modules is not supported and nft is a modules, we
-		 * need to use compat mode, which will not use CO-RE
-		 * read.
-		 */
-		trace_set_invalid(&trace_nft_do_chain);
-	} else {
-		trace_set_invalid(&trace_nft_do_chain_compat);
-	}
-
 	pr_debug("begin to resolve kernel symbol...\n");
 
 	/* make the programs that target kernel function can't be found
@@ -333,6 +349,9 @@ static int trace_prepare_traces()
 
 	pr_verb("following traces are enabled:\n");
 	trace_for_each(trace) {
+		if (trace_is_invalid(trace) || !trace_is_enable(trace))
+			continue;
+
 		if (trace_is_func(trace)) {
 			if (trace_is_ret(trace))
 				fmt = "kprobe/kretprobe";

src/trace.h

@@ -25,8 +25,9 @@ struct analyzer;
 #define TRACE_RET		(1 << 3)
 #define TRACE_STACK		(1 << 4)
 #define TRACE_ATTACH_MANUAL	(1 << 5)
+#define TRACE_CHECKED		(1 << 6)
 
-#define trace_for_each(pos) list_for_each_entry(pos, &trace_list, sibling)
+#define trace_for_each(pos) list_for_each_entry(pos, &trace_list, all)
 
 typedef struct trace_group {
 	char	*name;
@@ -44,19 +45,25 @@ typedef struct trace {
 	/* name of the eBPF program */
 	char	*prog;
 	enum trace_type type;
-	char	*if_str;
+	char	*cond;
 	char	*regex;
 	char	*tp;
 	int	skb;
-	struct list_head sibling;
+	/* traces in a global list */
+	struct list_head all;
+	/* traces in the same group */
 	struct list_head list;
+	/* list head of rules that belongs to this trace */
 	struct list_head rules;
+	/* traces that share the same target */
+	struct trace *sibling;
 	int	index;
 	u32	status;
 	trace_group_t *parent;
 	struct analyzer *analyzer;
 	/* if this trace should be enabled by default */
 	bool	def;
+	bool	mutex;
 } trace_t;
 
 typedef struct trace_args {

src/trace.yaml

@@ -51,9 +51,12 @@ children:
       - <<: *rule_ret_err
         msg: XDP failed to redirect skb
         adv: check if the target ifindex exist
-    - name: __netif_receive_skb_core:0
-    - name: __netif_receive_skb_core_pskb
+    - names:
+      - name: __netif_receive_skb_core_pskb
+        cond: '[ ! $(verlte "$(uname -r)" "5.4") -eq -1 ]' # valid when kernel >= 5.4
+      - name: __netif_receive_skb_core:0
       target: __netif_receive_skb_core
+      mutex: true
   - name: link-out
     desc: link layer (L2) of packet out
     visual: true
@@ -125,8 +128,13 @@ children:
     desc: base netfilter entry
     visual: true
     children:
-    - name: ipt_do_table
+    - names:
+      - name: ipt_do_table
+        cond: '[ $(verlte "$(uname -r)" "5.16") -eq -1 ]' # valid when kernel < 5.16
+      - name: ipt_do_table_new
       analyzer: iptable
+      mutex: true
+      target: ipt_do_table
       rules:
       - exp: eq 0
         level: error
@@ -135,11 +143,15 @@ children:
       - exp: eq 1
         level: info
         msg: packet is accepted
-    - name:
-      - nft_do_chain
+    - names:
+      - name: nft_do_chain
+        cond: >-
+          zcat /proc/config.gz | grep DEBUG_INFO_BTF_MODULES ||
+          cat /proc/kallsyms | grep " nft_do_chain$"
       - nft_do_chain_compat
       analyzer: iptable
       target: nft_do_chain
+      mutex: true
       rules:
       - exp: eq 0
         level: error