Skip to contentSkip to conversation

FAILED: patch "[PATCH] tracing/kprobes: Allow to create probe with a module name" failed to apply to 4.11-stable tree

PATCH [email protected] 4 participants 7 messages Jul 3, 2017 – Jul 7, 2017

Subsystems (2)
Stable branch Tracing
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.11-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 9e52b32567126fe146f198971364f68d3bc5233f Mon Sep 17 00:00:00 2001

From: Sabrina Dubroca <[email protected]>
Date: Thu, 22 Jun 2017 11:24:42 +0200
Subject: [PATCH] tracing/kprobes: Allow to create probe with a module name
starting with a digit

Always try to parse an address, since kstrtoul() will safely fail when given a symbol as input. If that fails (which will be the case for a symbol), try to parse a symbol instead.

This allows creating a probe such as:

    p:probe/vlan_gro_receive 8021q:vlan_gro_receive+0

Which is necessary for this command to work:

perf probe -m 8021q -a vlan_gro_receive

Link http://lkml.kernel.org/r/fd72d666f45b114e2c5b9cf7e27b91de1ec966f1.1498122881.git.sd@queasysnail.net open here
Cc [email protected]
Fixes 413d37d1e ("tracing: Add kprobe-based event tracer")
Acked-by Masami Hiramatsu <[email protected]>
Signed-off-by Sabrina Dubroca <[email protected]>
Signed-off-by Steven Rostedt (VMware) <[email protected]>
kernel/trace/trace_kprobe.c +5 -9
index c129fca6ec99..b53c8d369163 100644
@@ -707,20 +707,16 @@ static int create_trace_kprobe(int argc, char **argv)
 		pr_info("Probe point is not specified.\n");
 		return -EINVAL;
 	}
-	if (isdigit(argv[1][0])) {
-		/* an address specified */
-		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
-		if (ret) {
-			pr_info("Failed to parse address.\n");
-			return ret;
-		}
-	} else {
+
+	/* try to parse an address. if that fails, try to read the
+	 * input as a symbol. */
+	if (kstrtoul(argv[1], 0, (unsigned long *)&addr)) {
 		/* a symbol specified */
 		symbol = argv[1];
 		/* TODO: support .init module functions */
 		ret = traceprobe_split_symbol_offset(symbol, &offset);
 		if (ret) {
-			pr_info("Failed to parse symbol.\n");
+			pr_info("Failed to parse either an address or a symbol.\n");
 			return ret;
 		}
 		if (offset && is_return &&
maintainer
#1 lore raw

Masami and Sabrina,

I backported the patch to 4.11 as below. The one thing that changed here is that Linus's tree doesn't fail on return probes if the parameter is not a symbol. I didn't want to backport that to stable, so I modified the patch to retain that failure for return probes when an address is used.

Can you ack this patch and we can then have this be the backport for stable?

-- Steve

kernel/trace/trace_kprobe.c +8 -13
index 49cdda8..f0e99394 100644
@@ -680,30 +680,25 @@ static int create_trace_kprobe(int argc, char **argv)
 		pr_info("Probe point is not specified.\n");
 		return -EINVAL;
 	}
-	if (isdigit(argv[1][0])) {
-		if (is_return) {
-			pr_info("Return probe point must be a symbol.\n");
-			return -EINVAL;
-		}
-		/* an address specified */
-		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
-		if (ret) {
-			pr_info("Failed to parse address.\n");
-			return ret;
-		}
-	} else {
+
+	/* try to parse an address. if that fails, try to read the
+	 * input as a symbol. */
+	if (kstrtoul(argv[1], 0, (unsigned long *)&addr)) {
 		/* a symbol specified */
 		symbol = argv[1];
 		/* TODO: support .init module functions */
 		ret = traceprobe_split_symbol_offset(symbol, &offset);
 		if (ret) {
-			pr_info("Failed to parse symbol.\n");
+			pr_info("Failed to parse either an address or a symbol.\n");
 			return ret;
 		}
 		if (offset && is_return) {
 			pr_info("Return probe must be used without offset.\n");
 			return -EINVAL;
 		}
+	} else if (is_return) {
+		pr_info("Return probe point must be a symbol.\n");
+		return -EINVAL;
 	}
 	argc -= 2; argv += 2;
Acked-by
#2 lore raw
Quoted 10 lines · Steven Rostedt

Masami and Sabrina,

I backported the patch to 4.11 as below. The one thing that changed here is that Linus's tree doesn't fail on return probes if the parameter is not a symbol. I didn't want to backport that to stable, so I modified the patch to retain that failure for return probes when an address is used.

Can you ack this patch and we can then have this be the backport for stable?

Thanks, LGTM

Acked-by Sabrina Dubroca <[email protected]>
Signature · 1 line
Sabrina
maintainer
#3 lore raw
Quoted 9 lines · On Mon, 3 Jul 2017 16:28:32 +0200 Sabrina Dubroca <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt

Masami and Sabrina,

I backported the patch to 4.11 as below. The one thing that changed here is that Linus's tree doesn't fail on return probes if the parameter is not a symbol. I didn't want to backport that to stable, so I modified the patch to retain that failure for return probes when an address is used.

Quoted 7 lines
Quoted 2 lines

Can you ack this patch and we can then have this be the backport for stable?

Thanks, LGTM

Acked-by Sabrina Dubroca <[email protected]>

Thanks, I'll wait for Masami's ack, and then push it for Greg.

-- Steve

maintainer Acked-by
#4 lore raw
In reply to Steven Rostedt
Quoted 10 lines · On Mon, 3 Jul 2017 09:09:28 -0400 Steven Rostedt <[email protected]> wrote:

Masami and Sabrina,

I backported the patch to 4.11 as below. The one thing that changed here is that Linus's tree doesn't fail on return probes if the parameter is not a symbol. I didn't want to backport that to stable, so I modified the patch to retain that failure for return probes when an address is used.

Can you ack this patch and we can then have this be the backport for stable?

Looks good to me.

Acked-by Masami Hiramatsu <[email protected]>

Thank you!

Quoted 46 lines

-- Steve

kernel/trace/trace_kprobe.c +8 -13
index 49cdda8..f0e99394 100644
@@ -680,30 +680,25 @@ static int create_trace_kprobe(int argc, char **argv)
 		pr_info("Probe point is not specified.\n");
 		return -EINVAL;
 	}
-	if (isdigit(argv[1][0])) {
-		if (is_return) {
-			pr_info("Return probe point must be a symbol.\n");
-			return -EINVAL;
-		}
-		/* an address specified */
-		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
-		if (ret) {
-			pr_info("Failed to parse address.\n");
-			return ret;
-		}
-	} else {
+
+	/* try to parse an address. if that fails, try to read the
+	 * input as a symbol. */
+	if (kstrtoul(argv[1], 0, (unsigned long *)&addr)) {
 		/* a symbol specified */
 		symbol = argv[1];
 		/* TODO: support .init module functions */
 		ret = traceprobe_split_symbol_offset(symbol, &offset);
 		if (ret) {
-			pr_info("Failed to parse symbol.\n");
+			pr_info("Failed to parse either an address or a symbol.\n");
 			return ret;
 		}
 		if (offset && is_return) {
 			pr_info("Return probe must be used without offset.\n");
 			return -EINVAL;
 		}
+	} else if (is_return) {
+		pr_info("Return probe point must be a symbol.\n");
+		return -EINVAL;
 	}
 	argc -= 2; argv += 2;
Signature · 1 line
Masami Hiramatsu <[email protected]>
maintainer Acked-by Masami Hiramatsu
#5 lore raw
In reply to [email protected]

Greg,

Here's a backport of the failed patch. I got Sabrina's and Masami's acks for it. Can you please take this as the change. It applies all the way back to 3.18.

Thanks!

-- Steve

Quoted 1 line

From 9e52b32567126fe146f198971364f68d3bc5233f Mon Sep 17 00:00:00 2001

From: Sabrina Dubroca <[email protected]>
Date: Thu, 22 Jun 2017 11:24:42 +0200
Subject: [PATCH] tracing/kprobes: Allow to create probe with a module name
starting with a digit

Always try to parse an address, since kstrtoul() will safely fail when given a symbol as input. If that fails (which will be the case for a symbol), try to parse a symbol instead.

This allows creating a probe such as:

    p:probe/vlan_gro_receive 8021q:vlan_gro_receive+0

Which is necessary for this command to work:

perf probe -m 8021q -a vlan_gro_receive

Link http://lkml.kernel.org/r/fd72d666f45b114e2c5b9cf7e27b91de1ec966f1.1498122881.git.sd@queasysnail.net open here
Cc [email protected]
Fixes 413d37d1e ("tracing: Add kprobe-based event tracer")
Acked-by Masami Hiramatsu <[email protected]>
Signed-off-by Sabrina Dubroca <[email protected]>
Signed-off-by Steven Rostedt (VMware) <[email protected]>
kernel/trace/trace_kprobe.c +8 -13
Index: linux-trace.git/kernel/trace/trace_kprobe.c
===================================================================
@@ -680,30 +680,25 @@ static int create_trace_kprobe(int argc,
 		pr_info("Probe point is not specified.\n");
 		return -EINVAL;
 	}
-	if (isdigit(argv[1][0])) {
-		if (is_return) {
-			pr_info("Return probe point must be a symbol.\n");
-			return -EINVAL;
-		}
-		/* an address specified */
-		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
-		if (ret) {
-			pr_info("Failed to parse address.\n");
-			return ret;
-		}
-	} else {
+
+	/* try to parse an address. if that fails, try to read the
+	 * input as a symbol. */
+	if (kstrtoul(argv[1], 0, (unsigned long *)&addr)) {
 		/* a symbol specified */
 		symbol = argv[1];
 		/* TODO: support .init module functions */
 		ret = traceprobe_split_symbol_offset(symbol, &offset);
 		if (ret) {
-			pr_info("Failed to parse symbol.\n");
+			pr_info("Failed to parse either an address or a symbol.\n");
 			return ret;
 		}
 		if (offset && is_return) {
 			pr_info("Return probe must be used without offset.\n");
 			return -EINVAL;
 		}
+	} else if (is_return) {
+		pr_info("Return probe point must be a symbol.\n");
+		return -EINVAL;
 	}
 	argc -= 2; argv += 2;
author maintainer
#6 lore raw
Quoted 5 lines · Steven Rostedt

Greg,

Here's a backport of the failed patch. I got Sabrina's and Masami's acks for it. Can you please take this as the change. It applies all the way back to 3.18.

Thanks, now queued up.

greg k-h