lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3fdfffc5-6fd2-1fa4-c523-04333c538d49@oracle.com>
Date:   Wed, 11 Oct 2023 17:19:27 +0100
From:   Alan Maguire <alan.maguire@...cle.com>
To:     "Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Alexei Starovoitov <ast@...nel.org>
Cc:     linux-trace-kernel@...r.kernel.org, bpf@...r.kernel.org,
        linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH] bpf/btf: Move tracing BTF APIs to the BTF library

On 10/10/2023 14:54, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> 
> Move the BTF APIs used in tracing to the BTF library code for sharing it
> with others.
> Previously, to avoid complex dependency in a series I made it on the
> tracing tree, but now it is a good time to move it to BPF tree because
> these functions are pure BTF functions.
>

Makes sense to me. Two very small things - usual practice for
bpf-related changes is to specify "PATCH bpf-next" for changes like
this that target the -next tree. Other thing is I'm reasonably sure
no functional changes are intended - it's basically just a matter of
moving code from trace_btf -> btf - but would be good to confirm
that no functional changes are intended or similar in the commit
message. It's sort of implicit when you say "move the BTF APIs", but
would be good to confirm.


> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>

Reviewed-by: Alan Maguire <alan.maguire@...cle.com>


> ---
>  include/linux/btf.h        |   24 +++++++++
>  kernel/bpf/btf.c           |  115 +++++++++++++++++++++++++++++++++++++++++
>  kernel/trace/Makefile      |    1 
>  kernel/trace/trace_btf.c   |  122 --------------------------------------------
>  kernel/trace/trace_btf.h   |   11 ----
>  kernel/trace/trace_probe.c |    2 -
>  6 files changed, 140 insertions(+), 135 deletions(-)
>  delete mode 100644 kernel/trace/trace_btf.c
>  delete mode 100644 kernel/trace/trace_btf.h
> 
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 928113a80a95..8372d93ea402 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -507,6 +507,14 @@ btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
>  int get_kern_ctx_btf_id(struct bpf_verifier_log *log, enum bpf_prog_type prog_type);
>  bool btf_types_are_same(const struct btf *btf1, u32 id1,
>  			const struct btf *btf2, u32 id2);
> +const struct btf_type *btf_find_func_proto(const char *func_name,
> +					   struct btf **btf_p);
> +const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> +					   s32 *nr);
> +const struct btf_member *btf_find_struct_member(struct btf *btf,
> +						const struct btf_type *type,
> +						const char *member_name,
> +						u32 *anon_offset);
>  #else
>  static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
>  						    u32 type_id)
> @@ -559,6 +567,22 @@ static inline bool btf_types_are_same(const struct btf *btf1, u32 id1,
>  {
>  	return false;
>  }
> +static inline const struct btf_type *btf_find_func_proto(const char *func_name,
> +							 struct btf **btf_p)
> +{
> +	return NULL;
> +}
> +static inline const struct btf_param *
> +btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> +{
> +	return NULL;
> +}
> +static inline const struct btf_member *
> +btf_find_struct_member(struct btf *btf, const struct btf_type *type,
> +		       const char *member_name, u32 *anon_offset)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type *t)
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8090d7fb11ef..e5cbf3b31b78 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -912,6 +912,121 @@ static const struct btf_type *btf_type_skip_qualifiers(const struct btf *btf,
>  	return t;
>  }
>  
> +/*
> + * Find a function proto type by name, and return the btf_type with its btf
> + * in *@..._p. Return NULL if not found.
> + * Note that caller has to call btf_put(*@..._p) after using the btf_type.
> + */
> +const struct btf_type *btf_find_func_proto(const char *func_name, struct btf **btf_p)
> +{
> +	const struct btf_type *t;
> +	s32 id;
> +
> +	id = bpf_find_btf_id(func_name, BTF_KIND_FUNC, btf_p);
> +	if (id < 0)
> +		return NULL;
> +
> +	/* Get BTF_KIND_FUNC type */
> +	t = btf_type_by_id(*btf_p, id);
> +	if (!t || !btf_type_is_func(t))
> +		goto err;
> +
> +	/* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> +	t = btf_type_by_id(*btf_p, t->type);
> +	if (!t || !btf_type_is_func_proto(t))
> +		goto err;
> +
> +	return t;
> +err:
> +	btf_put(*btf_p);
> +	return NULL;
> +}
> +
> +/*
> + * Get function parameter with the number of parameters.
> + * This can return NULL if the function has no parameters.
> + * It can return -EINVAL if the @func_proto is not a function proto type.
> + */
> +const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> +{
> +	if (!btf_type_is_func_proto(func_proto))
> +		return ERR_PTR(-EINVAL);
> +
> +	*nr = btf_type_vlen(func_proto);
> +	if (*nr > 0)
> +		return (const struct btf_param *)(func_proto + 1);
> +	else
> +		return NULL;
> +}
> +
> +#define BTF_ANON_STACK_MAX	16
> +
> +struct btf_anon_stack {
> +	u32 tid;
> +	u32 offset;
> +};
> +
> +/*
> + * Find a member of data structure/union by name and return it.
> + * Return NULL if not found, or -EINVAL if parameter is invalid.
> + * If the member is an member of anonymous union/structure, the offset
> + * of that anonymous union/structure is stored into @anon_offset. Caller
> + * can calculate the correct offset from the root data structure by
> + * adding anon_offset to the member's offset.
> + */
> +const struct btf_member *btf_find_struct_member(struct btf *btf,
> +						const struct btf_type *type,
> +						const char *member_name,
> +						u32 *anon_offset)
> +{
> +	struct btf_anon_stack *anon_stack;
> +	const struct btf_member *member;
> +	u32 tid, cur_offset = 0;
> +	const char *name;
> +	int i, top = 0;
> +
> +	anon_stack = kcalloc(BTF_ANON_STACK_MAX, sizeof(*anon_stack), GFP_KERNEL);
> +	if (!anon_stack)
> +		return ERR_PTR(-ENOMEM);
> +
> +retry:
> +	if (!btf_type_is_struct(type)) {
> +		member = ERR_PTR(-EINVAL);
> +		goto out;
> +	}
> +
> +	for_each_member(i, type, member) {
> +		if (!member->name_off) {
> +			/* Anonymous union/struct: push it for later use */
> +			type = btf_type_skip_modifiers(btf, member->type, &tid);
> +			if (type && top < BTF_ANON_STACK_MAX) {
> +				anon_stack[top].tid = tid;
> +				anon_stack[top++].offset =
> +					cur_offset + member->offset;
> +			}
> +		} else {
> +			name = btf_name_by_offset(btf, member->name_off);
> +			if (name && !strcmp(member_name, name)) {
> +				if (anon_offset)
> +					*anon_offset = cur_offset;
> +				goto out;
> +			}
> +		}
> +	}
> +	if (top > 0) {
> +		/* Pop from the anonymous stack and retry */
> +		tid = anon_stack[--top].tid;
> +		cur_offset = anon_stack[top].offset;
> +		type = btf_type_by_id(btf, tid);
> +		goto retry;
> +	}
> +	member = NULL;
> +
> +out:
> +	kfree(anon_stack);
> +	return member;
> +}
> +
>  #define BTF_SHOW_MAX_ITER	10
>  
>  #define BTF_KIND_BIT(kind)	(1ULL << kind)
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index 057cd975d014..64b61f67a403 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -99,7 +99,6 @@ obj-$(CONFIG_KGDB_KDB) += trace_kdb.o
>  endif
>  obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
>  obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
> -obj-$(CONFIG_PROBE_EVENTS_BTF_ARGS) += trace_btf.o
>  obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
>  obj-$(CONFIG_BOOTTIME_TRACING) += trace_boot.o
>  obj-$(CONFIG_FTRACE_RECORD_RECURSION) += trace_recursion_record.o
> diff --git a/kernel/trace/trace_btf.c b/kernel/trace/trace_btf.c
> deleted file mode 100644
> index ca224d53bfdc..000000000000
> --- a/kernel/trace/trace_btf.c
> +++ /dev/null
> @@ -1,122 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -#include <linux/btf.h>
> -#include <linux/kernel.h>
> -#include <linux/slab.h>
> -
> -#include "trace_btf.h"
> -
> -/*
> - * Find a function proto type by name, and return the btf_type with its btf
> - * in *@..._p. Return NULL if not found.
> - * Note that caller has to call btf_put(*@..._p) after using the btf_type.
> - */
> -const struct btf_type *btf_find_func_proto(const char *func_name, struct btf **btf_p)
> -{
> -	const struct btf_type *t;
> -	s32 id;
> -
> -	id = bpf_find_btf_id(func_name, BTF_KIND_FUNC, btf_p);
> -	if (id < 0)
> -		return NULL;
> -
> -	/* Get BTF_KIND_FUNC type */
> -	t = btf_type_by_id(*btf_p, id);
> -	if (!t || !btf_type_is_func(t))
> -		goto err;
> -
> -	/* The type of BTF_KIND_FUNC is BTF_KIND_FUNC_PROTO */
> -	t = btf_type_by_id(*btf_p, t->type);
> -	if (!t || !btf_type_is_func_proto(t))
> -		goto err;
> -
> -	return t;
> -err:
> -	btf_put(*btf_p);
> -	return NULL;
> -}
> -
> -/*
> - * Get function parameter with the number of parameters.
> - * This can return NULL if the function has no parameters.
> - * It can return -EINVAL if the @func_proto is not a function proto type.
> - */
> -const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s32 *nr)
> -{
> -	if (!btf_type_is_func_proto(func_proto))
> -		return ERR_PTR(-EINVAL);
> -
> -	*nr = btf_type_vlen(func_proto);
> -	if (*nr > 0)
> -		return (const struct btf_param *)(func_proto + 1);
> -	else
> -		return NULL;
> -}
> -
> -#define BTF_ANON_STACK_MAX	16
> -
> -struct btf_anon_stack {
> -	u32 tid;
> -	u32 offset;
> -};
> -
> -/*
> - * Find a member of data structure/union by name and return it.
> - * Return NULL if not found, or -EINVAL if parameter is invalid.
> - * If the member is an member of anonymous union/structure, the offset
> - * of that anonymous union/structure is stored into @anon_offset. Caller
> - * can calculate the correct offset from the root data structure by
> - * adding anon_offset to the member's offset.
> - */
> -const struct btf_member *btf_find_struct_member(struct btf *btf,
> -						const struct btf_type *type,
> -						const char *member_name,
> -						u32 *anon_offset)
> -{
> -	struct btf_anon_stack *anon_stack;
> -	const struct btf_member *member;
> -	u32 tid, cur_offset = 0;
> -	const char *name;
> -	int i, top = 0;
> -
> -	anon_stack = kcalloc(BTF_ANON_STACK_MAX, sizeof(*anon_stack), GFP_KERNEL);
> -	if (!anon_stack)
> -		return ERR_PTR(-ENOMEM);
> -
> -retry:
> -	if (!btf_type_is_struct(type)) {
> -		member = ERR_PTR(-EINVAL);
> -		goto out;
> -	}
> -
> -	for_each_member(i, type, member) {
> -		if (!member->name_off) {
> -			/* Anonymous union/struct: push it for later use */
> -			type = btf_type_skip_modifiers(btf, member->type, &tid);
> -			if (type && top < BTF_ANON_STACK_MAX) {
> -				anon_stack[top].tid = tid;
> -				anon_stack[top++].offset =
> -					cur_offset + member->offset;
> -			}
> -		} else {
> -			name = btf_name_by_offset(btf, member->name_off);
> -			if (name && !strcmp(member_name, name)) {
> -				if (anon_offset)
> -					*anon_offset = cur_offset;
> -				goto out;
> -			}
> -		}
> -	}
> -	if (top > 0) {
> -		/* Pop from the anonymous stack and retry */
> -		tid = anon_stack[--top].tid;
> -		cur_offset = anon_stack[top].offset;
> -		type = btf_type_by_id(btf, tid);
> -		goto retry;
> -	}
> -	member = NULL;
> -
> -out:
> -	kfree(anon_stack);
> -	return member;
> -}
> -
> diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
> deleted file mode 100644
> index 4bc44bc261e6..000000000000
> --- a/kernel/trace/trace_btf.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#include <linux/btf.h>
> -
> -const struct btf_type *btf_find_func_proto(const char *func_name,
> -					   struct btf **btf_p);
> -const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> -					   s32 *nr);
> -const struct btf_member *btf_find_struct_member(struct btf *btf,
> -						const struct btf_type *type,
> -						const char *member_name,
> -						u32 *anon_offset);
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 4dc74d73fc1d..b33c424b8ee0 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -12,7 +12,7 @@
>  #define pr_fmt(fmt)	"trace_probe: " fmt
>  
>  #include <linux/bpf.h>
> -#include "trace_btf.h"
> +#include <linux/btf.h>
>  
>  #include "trace_probe.h"
>  
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ