diff mbox series

[v7,08/11] tracing/probes: Add BTF retval type support

Message ID 168255834145.2565678.12184411713423630481.stgit@mhiramat.roam.corp.google.com
State New
Headers
Series tracing: Add fprobe events |

Commit Message

Masami Hiramatsu (Google) April 27, 2023, 1:19 a.m. UTC 
  From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Check the target function has non-void retval type and set the correct
fetch type if user doesn't specify it.
If the function returns void, $retval is rejected as below;

 # echo 'f unregister_kprobes%return $retval' >> dynamic_events
sh: write error: No such file or directory
 # cat error_log
[   37.488397] trace_fprobe: error: This function returns 'void' type
  Command: f unregister_kprobes%return $retval
                                       ^

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v7:
 - Introduce this as a new patch.
---
 kernel/trace/trace_probe.c |   65 +++++++++++++++++++++++++++++++++++++++++---
 kernel/trace/trace_probe.h |    1 +
 2 files changed, 61 insertions(+), 5 deletions(-)

Comments

Masami Hiramatsu (Google) May 1, 2023, 10:11 a.m. UTC | #1 
On Thu, 27 Apr 2023 10:19:01 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Check the target function has non-void retval type and set the correct
> fetch type if user doesn't specify it.
> If the function returns void, $retval is rejected as below;
> 
>  # echo 'f unregister_kprobes%return $retval' >> dynamic_events
> sh: write error: No such file or directory
>  # cat error_log
> [   37.488397] trace_fprobe: error: This function returns 'void' type
>   Command: f unregister_kprobes%return $retval
>                                        ^
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> Changes in v7:
>  - Introduce this as a new patch.
> ---
>  kernel/trace/trace_probe.c |   65 +++++++++++++++++++++++++++++++++++++++++---
>  kernel/trace/trace_probe.h |    1 +
>  2 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 4c3c70862a9a..16d8edfe3d15 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -371,13 +371,13 @@ static const char *type_from_btf_id(struct btf *btf, s32 id)
>  	return NULL;
>  }
>  
> -static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr)
> +static const struct btf_type *find_btf_func_proto(const char *funcname)
>  {
>  	struct btf *btf = traceprobe_get_btf();
>  	const struct btf_type *t;
>  	s32 id;
>  
> -	if (!btf || !funcname || !nr)
> +	if (!btf || !funcname)
>  		return ERR_PTR(-EINVAL);
>  
>  	id = btf_find_by_name_kind(btf, funcname, BTF_KIND_FUNC);
> @@ -394,6 +394,20 @@ static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr
>  	if (!btf_type_is_func_proto(t))
>  		return ERR_PTR(-ENOENT);
>  
> +	return t;
> +}
> +
> +static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr)
> +{
> +	const struct btf_type *t;
> +
> +	if (!funcname || !nr)
> +		return ERR_PTR(-EINVAL);
> +
> +	t = find_btf_func_proto(funcname);
> +	if (IS_ERR(t))
> +		return (const struct btf_param *)t;
> +
>  	*nr = btf_type_vlen(t);
>  
>  	if (*nr)
> @@ -452,6 +466,32 @@ static const struct fetch_type *parse_btf_arg_type(int arg_idx,
>  	return find_fetch_type(typestr, ctx->flags);
>  }
>  
> +static const struct fetch_type *parse_btf_retval_type(
> +					struct traceprobe_parse_context *ctx)
> +{
> +	struct btf *btf = traceprobe_get_btf();
> +	const char *typestr = NULL;
> +	const struct btf_type *t;
> +
> +	if (btf && ctx->funcname) {
> +		t = find_btf_func_proto(ctx->funcname);
> +		if (!IS_ERR(t))
> +			typestr = type_from_btf_id(btf, t->type);
> +	}
> +
> +	return find_fetch_type(typestr, ctx->flags);
> +}
> +
> +static bool is_btf_retval_void(const char *funcname)
> +{
> +	const struct btf_type *t;
> +
> +	t = find_btf_func_proto(funcname);
> +	if (IS_ERR(t))
> +		return false;
> +
> +	return t->type == 0;
> +}
>  #else
>  static struct btf *traceprobe_get_btf(void)
>  {
> @@ -469,8 +509,15 @@ static int parse_btf_arg(const char *varname, struct fetch_insn *code,
>  	trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
>  	return -EOPNOTSUPP;
>  }
> +
>  #define parse_btf_arg_type(idx, ctx)		\
>  	find_fetch_type(NULL, ctx->flags)
> +
> +#define parse_btf_retval_type(ctx)		\
> +	find_fetch_type(NULL, ctx->flags)
> +
> +#define is_btf_retval_void(funcname)	(false)
> +
>  #endif
>  
>  #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
> @@ -500,7 +547,12 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
>  
>  	if (strcmp(arg, "retval") == 0) {
>  		if (ctx->flags & TPARG_FL_RETURN) {
> -			code->op = FETCH_OP_RETVAL;
> +			if ((ctx->flags & TPARG_FL_KERNEL) &&
> +			    is_btf_retval_void(ctx->funcname)) {
> +				trace_probe_log_err(ctx->offset, NO_RETVAL);
> +				ret = -ENOENT;
> +			} else
> +				code->op = FETCH_OP_RETVAL;
>  		} else {
>  			trace_probe_log_err(ctx->offset, RETVAL_ON_PROBE);
>  			ret = -EINVAL;
> @@ -887,9 +939,12 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
>  		goto fail;
>  
>  	/* Update storing type if BTF is available */
> -	if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) &&
> -	    !t && code->op == FETCH_OP_ARG)
> +	if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) && !t) {
> +	    if (code->op == FETCH_OP_ARG)
>  		parg->type = parse_btf_arg_type(code->param, ctx);
> +	    else if (code->op == FETCH_OP_RETVAL)
> +		parg->type = parse_btf_retval_type(ctx);
> +	}

Oops, here are wrong indents. I have to fix this.

Thanks,

>  
>  	ret = -EINVAL;
>  	/* Store operation */
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 8c5b029c5d62..3eb7c37c0984 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -444,6 +444,7 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
>  	C(BAD_EVENT_NAME,	"Event name must follow the same rules as C identifiers"), \
>  	C(EVENT_EXIST,		"Given group/event name is already used by another event"), \
>  	C(RETVAL_ON_PROBE,	"$retval is not available on probe"),	\
> +	C(NO_RETVAL,		"This function returns 'void' type"),	\
>  	C(BAD_STACK_NUM,	"Invalid stack number"),		\
>  	C(BAD_ARG_NUM,		"Invalid argument number"),		\
>  	C(BAD_VAR,		"Invalid $-valiable specified"),	\
>
diff mbox series

Patch

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 4c3c70862a9a..16d8edfe3d15 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -371,13 +371,13 @@  static const char *type_from_btf_id(struct btf *btf, s32 id)
 	return NULL;
 }
 
-static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr)
+static const struct btf_type *find_btf_func_proto(const char *funcname)
 {
 	struct btf *btf = traceprobe_get_btf();
 	const struct btf_type *t;
 	s32 id;
 
-	if (!btf || !funcname || !nr)
+	if (!btf || !funcname)
 		return ERR_PTR(-EINVAL);
 
 	id = btf_find_by_name_kind(btf, funcname, BTF_KIND_FUNC);
@@ -394,6 +394,20 @@  static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr
 	if (!btf_type_is_func_proto(t))
 		return ERR_PTR(-ENOENT);
 
+	return t;
+}
+
+static const struct btf_param *find_btf_func_param(const char *funcname, s32 *nr)
+{
+	const struct btf_type *t;
+
+	if (!funcname || !nr)
+		return ERR_PTR(-EINVAL);
+
+	t = find_btf_func_proto(funcname);
+	if (IS_ERR(t))
+		return (const struct btf_param *)t;
+
 	*nr = btf_type_vlen(t);
 
 	if (*nr)
@@ -452,6 +466,32 @@  static const struct fetch_type *parse_btf_arg_type(int arg_idx,
 	return find_fetch_type(typestr, ctx->flags);
 }
 
+static const struct fetch_type *parse_btf_retval_type(
+					struct traceprobe_parse_context *ctx)
+{
+	struct btf *btf = traceprobe_get_btf();
+	const char *typestr = NULL;
+	const struct btf_type *t;
+
+	if (btf && ctx->funcname) {
+		t = find_btf_func_proto(ctx->funcname);
+		if (!IS_ERR(t))
+			typestr = type_from_btf_id(btf, t->type);
+	}
+
+	return find_fetch_type(typestr, ctx->flags);
+}
+
+static bool is_btf_retval_void(const char *funcname)
+{
+	const struct btf_type *t;
+
+	t = find_btf_func_proto(funcname);
+	if (IS_ERR(t))
+		return false;
+
+	return t->type == 0;
+}
 #else
 static struct btf *traceprobe_get_btf(void)
 {
@@ -469,8 +509,15 @@  static int parse_btf_arg(const char *varname, struct fetch_insn *code,
 	trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
 	return -EOPNOTSUPP;
 }
+
 #define parse_btf_arg_type(idx, ctx)		\
 	find_fetch_type(NULL, ctx->flags)
+
+#define parse_btf_retval_type(ctx)		\
+	find_fetch_type(NULL, ctx->flags)
+
+#define is_btf_retval_void(funcname)	(false)
+
 #endif
 
 #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
@@ -500,7 +547,12 @@  static int parse_probe_vars(char *arg, const struct fetch_type *t,
 
 	if (strcmp(arg, "retval") == 0) {
 		if (ctx->flags & TPARG_FL_RETURN) {
-			code->op = FETCH_OP_RETVAL;
+			if ((ctx->flags & TPARG_FL_KERNEL) &&
+			    is_btf_retval_void(ctx->funcname)) {
+				trace_probe_log_err(ctx->offset, NO_RETVAL);
+				ret = -ENOENT;
+			} else
+				code->op = FETCH_OP_RETVAL;
 		} else {
 			trace_probe_log_err(ctx->offset, RETVAL_ON_PROBE);
 			ret = -EINVAL;
@@ -887,9 +939,12 @@  static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
 		goto fail;
 
 	/* Update storing type if BTF is available */
-	if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) &&
-	    !t && code->op == FETCH_OP_ARG)
+	if (IS_ENABLED(CONFIG_PROBE_EVENTS_BTF_ARGS) && !t) {
+	    if (code->op == FETCH_OP_ARG)
 		parg->type = parse_btf_arg_type(code->param, ctx);
+	    else if (code->op == FETCH_OP_RETVAL)
+		parg->type = parse_btf_retval_type(ctx);
+	}
 
 	ret = -EINVAL;
 	/* Store operation */
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 8c5b029c5d62..3eb7c37c0984 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -444,6 +444,7 @@  extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(BAD_EVENT_NAME,	"Event name must follow the same rules as C identifiers"), \
 	C(EVENT_EXIST,		"Given group/event name is already used by another event"), \
 	C(RETVAL_ON_PROBE,	"$retval is not available on probe"),	\
+	C(NO_RETVAL,		"This function returns 'void' type"),	\
 	C(BAD_STACK_NUM,	"Invalid stack number"),		\
 	C(BAD_ARG_NUM,		"Invalid argument number"),		\
 	C(BAD_VAR,		"Invalid $-valiable specified"),	\