* Improvements of BTF sanitizing for old kernels
@ 2025-04-07 19:01 Timur Chernykh
2025-04-07 19:01 ` [PATCH v2 1/2] libbpf: add proto_func param name generation Timur Chernykh
2025-04-07 19:01 ` [PATCH v2 2/2] libbpf: add kind flag sanitizing Timur Chernykh
0 siblings, 2 replies; 5+ messages in thread
From: Timur Chernykh @ 2025-04-07 19:01 UTC (permalink / raw)
To: bpf; +Cc: mykyta.yatsenko5
I've done fixing comments for first version of patch set (thanks for
Mykyta).
Here's a second version.
From: Timur Chernykh <tim.cherry.co@gmail.com>
To: bpf@vger.kernel.org
Cc: mykyta.yatsenko5@gmail.com
Bcc:
Reply-To: 20250331201016.345704-1-tim.cherry.co@gmail.com
Subject: Improvements of BTF sanitizing for old kernels
In-Reply-To:
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] libbpf: add proto_func param name generation
2025-04-07 19:01 Improvements of BTF sanitizing for old kernels Timur Chernykh
@ 2025-04-07 19:01 ` Timur Chernykh
2025-04-09 23:51 ` Andrii Nakryiko
2025-04-07 19:01 ` [PATCH v2 2/2] libbpf: add kind flag sanitizing Timur Chernykh
1 sibling, 1 reply; 5+ messages in thread
From: Timur Chernykh @ 2025-04-07 19:01 UTC (permalink / raw)
To: bpf; +Cc: mykyta.yatsenko5, Timur Chernykh
When the kernel loads BTF with specified min-CORE BTF and libbpf does some
sanitizing on those, then it "translates" func_proto to enum. But if
func_proto has no names for it's parameters then kernel verifier fails
with "Invalid name" error. This error caused by enum members must has a
valid C identifier, but there's might be no names generated in some
cases like function callback member declaration. This commit adds enum
names generation during sanitizing process for func_proto kind, when
it's being translate to `enum` kind.
Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
---
tools/lib/bpf/libbpf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6b85060f07b3..c2369b6f3260 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3128,6 +3128,8 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
+
+ char name_gen_buff[32] = {0};
int enum64_placeholder_id = 0;
struct btf_type *t;
int i, j, vlen;
@@ -3178,10 +3180,50 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
if (name[0] == '?')
name[0] = '_';
} else if (!has_func && btf_is_func_proto(t)) {
+ struct btf_param *params;
+ int new_name_off;
+
/* replace FUNC_PROTO with ENUM */
vlen = btf_vlen(t);
t->info = BTF_INFO_ENC(BTF_KIND_ENUM, 0, vlen);
t->size = sizeof(__u32); /* kernel enforced */
+
+ /* since the btf_enum and btf_param has the same binary layout
+ * it's ok to use btf_param
+ */
+ params = btf_params(t);
+
+ for (j = 0; j < vlen; ++j) {
+ struct btf_param *param = ¶ms[j];
+ const char *param_name = btf__str_by_offset(btf, param->name_off);
+
+ /*
+ * kernel disallow any unnamed enum members which can be generated for,
+ * as example, struct members like
+ * struct quota_format_ops {
+ * ...
+ * int (*get_next_id)(struct super_block *, struct kqid *);
+ * ...
+ * }
+ */
+ if (param_name && param_name[0])
+ continue; /* definitely has a name */
+
+ /*
+ * generate an uniq name for each func_proto
+ */
+ snprintf(name_gen_buff, sizeof(name_gen_buff), "__parm_proto_%d_%d", i, j);
+ new_name_off = btf__add_str(btf, name_gen_buff);
+
+ if (new_name_off < 0) {
+ pr_warn("Error creating the name for func_proto param\n");
+ return new_name_off;
+ }
+
+ /* give a valid name to func_proto param as it now an enum member */
+ param->name_off = new_name_off;
+ }
+
} else if (!has_func && btf_is_func(t)) {
/* replace FUNC with TYPEDEF */
t->info = BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0);
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] libbpf: add kind flag sanitizing
2025-04-07 19:01 Improvements of BTF sanitizing for old kernels Timur Chernykh
2025-04-07 19:01 ` [PATCH v2 1/2] libbpf: add proto_func param name generation Timur Chernykh
@ 2025-04-07 19:01 ` Timur Chernykh
2025-04-10 0:02 ` Andrii Nakryiko
1 sibling, 1 reply; 5+ messages in thread
From: Timur Chernykh @ 2025-04-07 19:01 UTC (permalink / raw)
To: bpf; +Cc: mykyta.yatsenko5, Timur Chernykh
Fix missed check whether kernel supports the kind flag or not.
The fix includes:
- The feature check whether kernel supports the kind flag or not
- Kind flag sanitizing if kernel doesn't support one
- Struct/enum bitfield members sanitizing by generation a proper
replacement for the type of bitfield with corresponding integer
type with same bit size
Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
---
tools/lib/bpf/features.c | 30 +++++++++++++
tools/lib/bpf/libbpf.c | 74 ++++++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf_internal.h | 2 +
3 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index 760657f5224c..b40a3fadb68b 100644
--- a/tools/lib/bpf/features.c
+++ b/tools/lib/bpf/features.c
@@ -507,6 +507,33 @@ static int probe_kern_arg_ctx_tag(int token_fd)
return probe_fd(prog_fd);
}
+static int probe_kern_btf_type_kind_flag(int token_fd)
+{
+ static const char strs[] = "\0bpf_spin_lock\0val\0cnt\0l";
+ /* struct bpf_spin_lock {
+ * int val;
+ * };
+ * struct val {
+ * int cnt;
+ * struct bpf_spin_lock l;
+ * };
+ */
+ __u32 types[] = {
+ /* int */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ /* struct bpf_spin_lock */ /* [2] */
+ BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_STRUCT, 1 /* kind bit */, 1), 4),
+ BTF_MEMBER_ENC(15, 1, 0), /* int val; */
+ /* struct val */ /* [3] */
+ BTF_TYPE_ENC(15, BTF_INFO_ENC(BTF_KIND_STRUCT, 1 /* kind bit */, 2), 8),
+ BTF_MEMBER_ENC(19, 1, 0), /* int cnt; */
+ BTF_MEMBER_ENC(23, 2, 32),/* struct bpf_spin_lock l; */
+ };
+
+ return probe_fd(libbpf__load_raw_btf((char *)types, sizeof(types),
+ strs, sizeof(strs), token_fd));
+}
+
typedef int (*feature_probe_fn)(int /* token_fd */);
static struct kern_feature_cache feature_cache;
@@ -582,6 +609,9 @@ static struct kern_feature_desc {
[FEAT_BTF_QMARK_DATASEC] = {
"BTF DATASEC names starting from '?'", probe_kern_btf_qmark_datasec,
},
+ [FEAT_BTF_TYPE_KIND_FLAG] = {
+ "BTF btf_type can have the kind flags set", probe_kern_btf_type_kind_flag,
+ },
};
bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c2369b6f3260..b1d4530bd9ed 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3113,9 +3113,11 @@ static bool btf_needs_sanitization(struct bpf_object *obj)
bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
+ bool has_kind_bit_support = kernel_supports(obj, FEAT_BTF_TYPE_KIND_FLAG);
return !has_func || !has_datasec || !has_func_global || !has_float ||
- !has_decl_tag || !has_type_tag || !has_enum64 || !has_qmark_datasec;
+ !has_decl_tag || !has_type_tag || !has_enum64 || !has_qmark_datasec ||
+ !has_kind_bit_support;
}
static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
@@ -3128,6 +3130,7 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
+ bool has_kind_bit_support = kernel_supports(obj, FEAT_BTF_TYPE_KIND_FLAG);
char name_gen_buff[32] = {0};
int enum64_placeholder_id = 0;
@@ -3263,6 +3266,75 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
m->type = enum64_placeholder_id;
m->offset = 0;
}
+ } else if (!has_kind_bit_support &&
+ (btf_is_composite(t) || btf_is_fwd(t) || btf_is_enum(t) || btf_is_enum64(t))) {
+ vlen = btf_vlen(t);
+
+ /* type encoded with a kind flag */
+ if (btf_kflag(t))
+ continue;
+
+ /* unset kind flag anyway */
+ t->info = BTF_INFO_ENC(btf_kind(t), 0, btf_vlen(t));
+
+ /* compisite types has a different bitfield processing if kind flag is set */
+ if (btf_is_composite(t)) {
+ struct btf_member *members = btf_members(t);
+
+ struct btf_type *curr_type = NULL; /* current member type */
+ struct btf_type *new_type = NULL; /* replacement for current member type */
+ int curr_tid = 0;
+ int new_tid = 0;
+ __u32 *new_type_data = NULL;
+ int encoding = 0;
+
+ for (j = 0; j < vlen; j++) {
+ struct btf_member *member = &members[j];
+
+ /* unwrap typedefs, volatiles, etc. */
+ curr_tid = btf__resolve_type(btf, member->type);
+
+ if (curr_tid < 0) {
+ pr_warn("Error resolving type [%d] for member %d of [%d]\n",
+ member->type, j, i);
+ return curr_tid;
+ }
+
+ curr_type = btf_type_by_id(btf, curr_tid);
+
+ /* bitfields can be only int or enum values */
+ if (!(btf_is_int(curr_type) || btf_is_enum(curr_type)))
+ continue;
+
+ encoding = btf_int_encoding(curr_type);
+
+ /* enum value encodes integer signed/unsigned info in the kind flag */
+ if (btf_is_enum(curr_type) && btf_kflag(curr_type))
+ encoding = BTF_INT_SIGNED;
+
+ /* create new integral type with the same info */
+ snprintf(name_gen_buff, sizeof(name_gen_buff), "__int_%d_%d", i, j);
+ new_tid = btf__add_int(btf, name_gen_buff, curr_type->size, encoding);
+
+ if (new_tid < 0) {
+ pr_warn("Error adding integer type for a bitfield %d of [%d]\n", j, i);
+ return new_tid;
+ }
+
+ new_type = btf_type_by_id(btf, new_tid);
+
+ /* encode int in legacy way,
+ * keep offset 0 and specify bit size as set in the member
+ */
+ new_type_data = (__u32 *)(new_type + 1);
+ *new_type_data = BTF_INT_ENC(encoding, 0,
+ BTF_MEMBER_BITFIELD_SIZE(member->offset));
+
+ /* old kernels looks only on offset */
+ member->offset = BTF_MEMBER_BIT_OFFSET(member->offset);
+ member->type = new_tid;
+ }
+ }
}
}
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 76669c73dcd1..6369c5520fce 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -380,6 +380,8 @@ enum kern_feature_id {
FEAT_ARG_CTX_TAG,
/* Kernel supports '?' at the front of datasec names */
FEAT_BTF_QMARK_DATASEC,
+ /* Kernel supports kind flag */
+ FEAT_BTF_TYPE_KIND_FLAG,
__FEAT_CNT,
};
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] libbpf: add proto_func param name generation
2025-04-07 19:01 ` [PATCH v2 1/2] libbpf: add proto_func param name generation Timur Chernykh
@ 2025-04-09 23:51 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2025-04-09 23:51 UTC (permalink / raw)
To: 20250331201016.345704-1-tim.cherry.co
Cc: bpf, mykyta.yatsenko5, Timur Chernykh
On Mon, Apr 7, 2025 at 12:24 PM Timur Chernykh <tim.cherry.co@gmail.com> wrote:
>
> When the kernel loads BTF with specified min-CORE BTF and libbpf does some
> sanitizing on those, then it "translates" func_proto to enum. But if
> func_proto has no names for it's parameters then kernel verifier fails
> with "Invalid name" error. This error caused by enum members must has a
> valid C identifier, but there's might be no names generated in some
> cases like function callback member declaration. This commit adds enum
> names generation during sanitizing process for func_proto kind, when
> it's being translate to `enum` kind.
>
I asked ChatGPT to fix up grammar and typos, it did a pretty good job,
actually :)
When the kernel loads BTF with a specified min-CORE BTF and libbpf
performs some sanitization, it "translates" FUNC_PROTO to an ENUM.
However, if the FUNC_PROTO has no parameter names, the kernel verifier
fails with an "Invalid name" error. This is because enum members must
have valid C identifiers, but in some cases—such as function callback
member declarations—no names may be generated.
This commit adds name generation for enum members during the
sanitization process of the FUNC_PROTO kind when it is being
translated to the ENUM kind.
> Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
> ---
> tools/lib/bpf/libbpf.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6b85060f07b3..c2369b6f3260 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3128,6 +3128,8 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
> bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
> bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
> +
don't split variable declaration block with empty lines
> + char name_gen_buff[32] = {0};
> int enum64_placeholder_id = 0;
> struct btf_type *t;
> int i, j, vlen;
> @@ -3178,10 +3180,50 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> if (name[0] == '?')
> name[0] = '_';
> } else if (!has_func && btf_is_func_proto(t)) {
> + struct btf_param *params;
> + int new_name_off;
> +
> /* replace FUNC_PROTO with ENUM */
> vlen = btf_vlen(t);
> t->info = BTF_INFO_ENC(BTF_KIND_ENUM, 0, vlen);
> t->size = sizeof(__u32); /* kernel enforced */
> +
> + /* since the btf_enum and btf_param has the same binary layout
> + * it's ok to use btf_param
> + */
> + params = btf_params(t);
> +
> + for (j = 0; j < vlen; ++j) {
> + struct btf_param *param = ¶ms[j];
> + const char *param_name = btf__str_by_offset(btf, param->name_off);
> +
> + /*
> + * kernel disallow any unnamed enum members which can be generated for,
> + * as example, struct members like
> + * struct quota_format_ops {
> + * ...
> + * int (*get_next_id)(struct super_block *, struct kqid *);
> + * ...
> + * }
> + */
> + if (param_name && param_name[0])
> + continue; /* definitely has a name */
> +
> + /*
> + * generate an uniq name for each func_proto
> + */
> + snprintf(name_gen_buff, sizeof(name_gen_buff), "__parm_proto_%d_%d", i, j);
> + new_name_off = btf__add_str(btf, name_gen_buff);
> +
> + if (new_name_off < 0) {
> + pr_warn("Error creating the name for func_proto param\n");
> + return new_name_off;
> + }
> +
> + /* give a valid name to func_proto param as it now an enum member */
> + param->name_off = new_name_off;
kernel doesn't really check nor enforce that enumerator names should
be unique, just that they are valid, so instead of all this we can
just add a trivial "p" string and use it for all parameters
try to make this code a bit more tight and succinct: shorter names
(look around the code you are adding new code to and keep the styling
consistent), no need for pr_warn(), it's unlikely that we'll fail to
add a string (we don't explicitly log -ENOMEM conditions)
also, instead of param = ¶ms[j] and so on, just do
struct btf_param *p;
p = btf_params(t);
for (j = 0; j < vlen; j++, p++) { ... }
pw-bot: cr
> + }
> +
> } else if (!has_func && btf_is_func(t)) {
> /* replace FUNC with TYPEDEF */
> t->info = BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0);
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] libbpf: add kind flag sanitizing
2025-04-07 19:01 ` [PATCH v2 2/2] libbpf: add kind flag sanitizing Timur Chernykh
@ 2025-04-10 0:02 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2025-04-10 0:02 UTC (permalink / raw)
To: 20250331201016.345704-1-tim.cherry.co
Cc: bpf, mykyta.yatsenko5, Timur Chernykh
On Mon, Apr 7, 2025 at 12:25 PM Timur Chernykh <tim.cherry.co@gmail.com> wrote:
>
> Fix missed check whether kernel supports the kind flag or not.
> The fix includes:
> - The feature check whether kernel supports the kind flag or not
> - Kind flag sanitizing if kernel doesn't support one
> - Struct/enum bitfield members sanitizing by generation a proper
> replacement for the type of bitfield with corresponding integer
> type with same bit size
>
> Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
> ---
> tools/lib/bpf/features.c | 30 +++++++++++++
> tools/lib/bpf/libbpf.c | 74 ++++++++++++++++++++++++++++++++-
> tools/lib/bpf/libbpf_internal.h | 2 +
> 3 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index 760657f5224c..b40a3fadb68b 100644
> --- a/tools/lib/bpf/features.c
> +++ b/tools/lib/bpf/features.c
> @@ -507,6 +507,33 @@ static int probe_kern_arg_ctx_tag(int token_fd)
> return probe_fd(prog_fd);
> }
>
> +static int probe_kern_btf_type_kind_flag(int token_fd)
> +{
> + static const char strs[] = "\0bpf_spin_lock\0val\0cnt\0l";
> + /* struct bpf_spin_lock {
> + * int val;
> + * };
> + * struct val {
> + * int cnt;
> + * struct bpf_spin_lock l;
> + * };
> + */
> + __u32 types[] = {
> + /* int */
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> + /* struct bpf_spin_lock */ /* [2] */
> + BTF_TYPE_ENC(1, BTF_INFO_ENC(BTF_KIND_STRUCT, 1 /* kind bit */, 1), 4),
> + BTF_MEMBER_ENC(15, 1, 0), /* int val; */
> + /* struct val */ /* [3] */
> + BTF_TYPE_ENC(15, BTF_INFO_ENC(BTF_KIND_STRUCT, 1 /* kind bit */, 2), 8),
> + BTF_MEMBER_ENC(19, 1, 0), /* int cnt; */
> + BTF_MEMBER_ENC(23, 2, 32),/* struct bpf_spin_lock l; */
> + };
this is a feature probe, no need to use "real" type names, there is
nothing special about bpf_spin_lock, so it can be just "s", for
example
also, why do you have two structs here? one with kflag=1 wouldn't be enough?
> +
> + return probe_fd(libbpf__load_raw_btf((char *)types, sizeof(types),
> + strs, sizeof(strs), token_fd));
> +}
> +
> typedef int (*feature_probe_fn)(int /* token_fd */);
>
> static struct kern_feature_cache feature_cache;
> @@ -582,6 +609,9 @@ static struct kern_feature_desc {
> [FEAT_BTF_QMARK_DATASEC] = {
> "BTF DATASEC names starting from '?'", probe_kern_btf_qmark_datasec,
> },
> + [FEAT_BTF_TYPE_KIND_FLAG] = {
> + "BTF btf_type can have the kind flags set", probe_kern_btf_type_kind_flag,
"BTF struct kflag support" ?
> + },
> };
>
> bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id)
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index c2369b6f3260..b1d4530bd9ed 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3113,9 +3113,11 @@ static bool btf_needs_sanitization(struct bpf_object *obj)
> bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
> bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
> bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
> + bool has_kind_bit_support = kernel_supports(obj, FEAT_BTF_TYPE_KIND_FLAG);
has_kind_flag, no _support. Do you see "_support" in any other similar variable?
>
> return !has_func || !has_datasec || !has_func_global || !has_float ||
> - !has_decl_tag || !has_type_tag || !has_enum64 || !has_qmark_datasec;
> + !has_decl_tag || !has_type_tag || !has_enum64 || !has_qmark_datasec ||
> + !has_kind_bit_support;
looks like something is off with indentation, please check
> }
>
> static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> @@ -3128,6 +3130,7 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
> bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
> bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
> + bool has_kind_bit_support = kernel_supports(obj, FEAT_BTF_TYPE_KIND_FLAG);
>
> char name_gen_buff[32] = {0};
> int enum64_placeholder_id = 0;
> @@ -3263,6 +3266,75 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
> m->type = enum64_placeholder_id;
> m->offset = 0;
> }
> + } else if (!has_kind_bit_support &&
> + (btf_is_composite(t) || btf_is_fwd(t) || btf_is_enum(t) || btf_is_enum64(t))) {
hm... kflag for structs, fwds, and enums I think were added at
different times, so they'd need to be detected separately, I think...
> + vlen = btf_vlen(t);
> +
> + /* type encoded with a kind flag */
> + if (btf_kflag(t))
> + continue;
> +
> + /* unset kind flag anyway */
> + t->info = BTF_INFO_ENC(btf_kind(t), 0, btf_vlen(t));
> +
> + /* compisite types has a different bitfield processing if kind flag is set */
indentation is off
typo: composite
> + if (btf_is_composite(t)) {
> + struct btf_member *members = btf_members(t);
> +
no empty line here
> + struct btf_type *curr_type = NULL; /* current member type */
curr -> cur, there is barely any "curr" usage in libbpf code base (one
straggler in ringbuf.c, sigh)
> + struct btf_type *new_type = NULL; /* replacement for current member type */
> + int curr_tid = 0;
> + int new_tid = 0;
> + __u32 *new_type_data = NULL;
> + int encoding = 0;
> +
> + for (j = 0; j < vlen; j++) {
> + struct btf_member *member = &members[j];
> +
> + /* unwrap typedefs, volatiles, etc. */
> + curr_tid = btf__resolve_type(btf, member->type);
> +
> + if (curr_tid < 0) {
> + pr_warn("Error resolving type [%d] for member %d of [%d]\n",
> + member->type, j, i);
> + return curr_tid;
> + }
> +
> + curr_type = btf_type_by_id(btf, curr_tid);
> +
> + /* bitfields can be only int or enum values */
> + if (!(btf_is_int(curr_type) || btf_is_enum(curr_type)))
> + continue;
> +
> + encoding = btf_int_encoding(curr_type);
> +
> + /* enum value encodes integer signed/unsigned info in the kind flag */
> + if (btf_is_enum(curr_type) && btf_kflag(curr_type))
> + encoding = BTF_INT_SIGNED;
> +
> + /* create new integral type with the same info */
> + snprintf(name_gen_buff, sizeof(name_gen_buff), "__int_%d_%d", i, j);
> + new_tid = btf__add_int(btf, name_gen_buff, curr_type->size, encoding);
> +
> + if (new_tid < 0) {
> + pr_warn("Error adding integer type for a bitfield %d of [%d]\n", j, i);
> + return new_tid;
> + }
> +
> + new_type = btf_type_by_id(btf, new_tid);
> +
> + /* encode int in legacy way,
> + * keep offset 0 and specify bit size as set in the member
> + */
> + new_type_data = (__u32 *)(new_type + 1);
> + *new_type_data = BTF_INT_ENC(encoding, 0,
> + BTF_MEMBER_BITFIELD_SIZE(member->offset));
> +
> + /* old kernels looks only on offset */
> + member->offset = BTF_MEMBER_BIT_OFFSET(member->offset);
> + member->type = new_tid;
this seems like a huge overkill, tbh... can't we do something
much-much simpler? e.g., just reset all offsets to their byte-aligned
equivalent? or reset them all to zero? Worst case, we can turn STRUCT
into UNION.
> + }
> + }
> }
> }
>
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 76669c73dcd1..6369c5520fce 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -380,6 +380,8 @@ enum kern_feature_id {
> FEAT_ARG_CTX_TAG,
> /* Kernel supports '?' at the front of datasec names */
> FEAT_BTF_QMARK_DATASEC,
> + /* Kernel supports kind flag */
> + FEAT_BTF_TYPE_KIND_FLAG,
> __FEAT_CNT,
> };
>
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-10 0:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 19:01 Improvements of BTF sanitizing for old kernels Timur Chernykh
2025-04-07 19:01 ` [PATCH v2 1/2] libbpf: add proto_func param name generation Timur Chernykh
2025-04-09 23:51 ` Andrii Nakryiko
2025-04-07 19:01 ` [PATCH v2 2/2] libbpf: add kind flag sanitizing Timur Chernykh
2025-04-10 0:02 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox