* Improvements of BTF sanitizing for old kernels
@ 2025-03-31 20:09 Timur Chernykh
2025-03-31 20:09 ` [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type Timur Chernykh
2025-03-31 20:09 ` [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not Timur Chernykh
0 siblings, 2 replies; 6+ messages in thread
From: Timur Chernykh @ 2025-03-31 20:09 UTC (permalink / raw)
To: bpf
Hello everyone. Recently I've encountered some issues with loading on debian 10 with kernel version `4.19.0-[17-22]`.
This first one consists of when the kernel loads BTF with specified min-CORE 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.
The second issue this PR supposed to fix is missed check whether kernel supports the `kind flag` or not. Let's say we're compiling the BPF object on new kernel with a new compiler. There's 99% percent chance that compiler will use this `kind flag`. But of we're loading on old kernels that have no support of this, so the loading will fail in multiple places.
- The type metadata check. Old BTF_INFO_MASK prohibits 31d bit
- Integer bitfield verification. Because old kernel waits for an offset-only encoding for the btf_member->offset field, but it's encoded in different way in case when the `kind flag` is set.
This PR contains:
- Enum names generation during sanitizing process for `func_proto` kind, when it being translate to `enum` kind.
- 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 the type of bitfield with corresponding integer type with same bit size
From: Timur Chernykh <tim.cherry.co@gmail.com>
To: bpf@vger.kernel.org
Cc:
Bcc:
Reply-To:
Subject: Improvements of BTF sanitizing for old kernels
In-Reply-To:
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type
2025-03-31 20:09 Improvements of BTF sanitizing for old kernels Timur Chernykh
@ 2025-03-31 20:09 ` Timur Chernykh
2025-04-02 12:41 ` Mykyta Yatsenko
2025-03-31 20:09 ` [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not Timur Chernykh
1 sibling, 1 reply; 6+ messages in thread
From: Timur Chernykh @ 2025-03-31 20:09 UTC (permalink / raw)
To: bpf; +Cc: Timur Chernykh
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..8e1edba443dd 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_param_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]) {
+ /* definitely has a name, valid it or no should decide kernel verifier */
+ continue;
+ }
+
+ /*
+ * generate an uniq name for each func_proto
+ */
+ snprintf(name_gen_buff, sizeof(name_gen_buff), "__parm_proto_%d_%d", i, j);
+ new_param_name_off = btf__add_str(btf, name_gen_buff);
+
+ if (new_param_name_off < 0) {
+ pr_warn("Error creating the name for func_proto param");
+ return new_param_name_off;
+ }
+
+ /* give a valid name to func_proto param as it now an enum member */
+ param->name_off = new_param_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] 6+ messages in thread
* [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not
2025-03-31 20:09 Improvements of BTF sanitizing for old kernels Timur Chernykh
2025-03-31 20:09 ` [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type Timur Chernykh
@ 2025-03-31 20:09 ` Timur Chernykh
2025-04-02 13:05 ` Mykyta Yatsenko
1 sibling, 1 reply; 6+ messages in thread
From: Timur Chernykh @ 2025-03-31 20:09 UTC (permalink / raw)
To: bpf; +Cc: Timur Chernykh
Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
---
tools/lib/bpf/features.c | 30 ++++++++++++++++
tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf_internal.h | 2 ++
3 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index 760657f5224c..dfab65f30f0c 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)
+{
+ 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 8e1edba443dd..392779c10a73 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3113,9 +3113,10 @@ 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 +3129,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 +3265,64 @@ 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_struct(t) || btf_is_union(t) || btf_is_fwd(t) || btf_is_enum(t) || btf_is_enum64(t))) {
+ const uint16_t members_cnt = btf_vlen(t);
+
+ /* type encoded with a kind flag */
+ if (t->info != BTF_INFO_ENC(btf_kind(t), 1, members_cnt)) {
+ continue;
+ }
+
+ /* unset kind flag anyway */
+ t->info = BTF_INFO_ENC(btf_kind(t), 0, btf_vlen(t));
+
+ /* structs an unions has a different bitfield processing behaviour is kind flag is set */
+ if (btf_is_struct(t) || btf_is_union(t)) {
+ struct btf_member* members = btf_members(t);
+ struct btf_type* new_int_type = NULL;
+ int new_int_type_id;
+ __u32* new_int_type_data;
+ int encoding = 0;
+ int nmember;
+
+ for (nmember = 0; nmember < members_cnt; nmember++) {
+ struct btf_member* member = &members[nmember];
+ const struct btf_type* member_type = btf_type_by_id(btf, member->type);
+
+ while (btf_is_typedef(member_type)) { /* unwrap typedefs */
+ member_type = btf_type_by_id(btf, member_type->type);
+ }
+
+ /* bitfields can be only int or enum values */
+ if (!(btf_is_int(member_type) || btf_is_enum(member_type))) {
+ continue;
+ }
+
+ encoding = btf_int_encoding(member_type);
+ if (btf_is_enum(member_type) && member_type->info & 0x80000000 /* kind flag */) {
+ /* enum value encodes integer signed/unsigned info in the kind flag */
+ encoding = BTF_INT_SIGNED;
+ }
+
+ /* create new integral type with the same info */
+ snprintf(name_gen_buff, sizeof(name_gen_buff), "__int_%d_%d", i, nmember);
+ new_int_type_id = btf__add_int(btf, name_gen_buff, member_type->size, encoding);
+
+ if (new_int_type_id < 0) {
+ pr_warn("Error adding integer type for a bitfield %d of [%d]", nmember, i);
+ return new_int_type_id;
+ }
+
+ new_int_type = btf_type_by_id(btf, new_int_type_id);
+
+ /* encode int in legacy way, keep offset 0 and specify bit size as set in the member */
+ new_int_type_data = (__u32*)(new_int_type + 1);
+ *new_int_type_data = BTF_INT_ENC(encoding, 0, BTF_MEMBER_BITFIELD_SIZE(member->offset));
+
+ member->type = new_int_type_id;
+ member->offset = BTF_MEMBER_BIT_OFFSET(member->offset) /* old kernels looks only on offset */;
+ }
+ }
}
}
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] 6+ messages in thread
* [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not
2025-03-31 20:45 Improvements of BTF sanitizing for old kernels Timur Chernykh
@ 2025-03-31 20:45 ` Timur Chernykh
0 siblings, 0 replies; 6+ messages in thread
From: Timur Chernykh @ 2025-03-31 20:45 UTC (permalink / raw)
To: bpf; +Cc: Timur Chernykh
Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
---
tools/lib/bpf/features.c | 30 ++++++++++++++++
tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++++++++++++++-
tools/lib/bpf/libbpf_internal.h | 2 ++
3 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index 760657f5224c..dfab65f30f0c 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)
+{
+ 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 8e1edba443dd..13ed8f03cc65 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3113,9 +3113,10 @@ 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 +3129,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 +3265,64 @@ 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_struct(t) || btf_is_union(t) || btf_is_fwd(t) || btf_is_enum(t) || btf_is_enum64(t))) {
+ const uint16_t members_cnt = btf_vlen(t);
+
+ /* type encoded with a kind flag */
+ if (t->info != BTF_INFO_ENC(btf_kind(t), 1, members_cnt)) {
+ continue;
+ }
+
+ /* unset kind flag anyway */
+ t->info = BTF_INFO_ENC(btf_kind(t), 0, btf_vlen(t));
+
+ /* structs an unions has a different bitfield processing behaviour is kind flag is set */
+ if (btf_is_struct(t) || btf_is_union(t)) {
+ struct btf_member* members = btf_members(t);
+ struct btf_type* new_int_type = NULL;
+ int new_int_type_id;
+ __u32* new_int_type_data;
+ int encoding = 0;
+ int nmember;
+
+ for (nmember = 0; nmember < members_cnt; nmember++) {
+ struct btf_member* member = &members[nmember];
+ const struct btf_type* member_type = btf_type_by_id(btf, member->type);
+
+ while (btf_is_typedef(member_type)) { /* unwrap typedefs */
+ member_type = btf_type_by_id(btf, member_type->type);
+ }
+
+ /* bitfields can be only int or enum values */
+ if (!(btf_is_int(member_type) || btf_is_enum(member_type))) {
+ continue;
+ }
+
+ encoding = btf_int_encoding(member_type);
+ if (btf_is_enum(member_type) && member_type->info & 0x80000000 /* kind flag */) {
+ /* enum value encodes integer signed/unsigned info in the kind flag */
+ encoding = BTF_INT_SIGNED;
+ }
+
+ /* create new integral type with the same info */
+ snprintf(name_gen_buff, sizeof(name_gen_buff), "__int_%d_%d", i, nmember);
+ new_int_type_id = btf__add_int(btf, name_gen_buff, member_type->size, encoding);
+
+ if (new_int_type_id < 0) {
+ pr_warn("Error adding integer type for a bitfield %d of [%d]", nmember, i);
+ return new_int_type_id;
+ }
+
+ new_int_type = btf_type_by_id(btf, new_int_type_id);
+
+ /* encode int in legacy way, keep offset 0 and specify bit size as set in the member */
+ new_int_type_data = (__u32*)(new_int_type + 1);
+ *new_int_type_data = BTF_INT_ENC(encoding, 0, BTF_MEMBER_BITFIELD_SIZE(member->offset));
+
+ member->type = new_int_type_id;
+ member->offset = BTF_MEMBER_BIT_OFFSET(member->offset) /* old kernels looks only on offset */;
+ }
+ }
}
}
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] 6+ messages in thread
* Re: [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type
2025-03-31 20:09 ` [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type Timur Chernykh
@ 2025-04-02 12:41 ` Mykyta Yatsenko
0 siblings, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2025-04-02 12:41 UTC (permalink / raw)
To: Timur Chernykh, bpf
On 31/03/2025 21:09, Timur Chernykh wrote:
> Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
Thanks for submitting this patchset, do you mind adding a proper commit
message for each commit in the set.
> ---
> 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..8e1edba443dd 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;
`struct btf_param *params` here and in other places, put asterisk closer
to var name, not type.
> + int new_param_name_off;
Maybe just `new_name_off`, prefer shorter names, when possible.
> +
> /* 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]) {
> + /* definitely has a name, valid it or no should decide kernel verifier */
> + continue;
> + }
> +
> + /*
> + * generate an uniq name for each func_proto
> + */
> + snprintf(name_gen_buff, sizeof(name_gen_buff), "__parm_proto_%d_%d", i, j);
> + new_param_name_off = btf__add_str(btf, name_gen_buff);
> +
> + if (new_param_name_off < 0) {
> + pr_warn("Error creating the name for func_proto param");
append \n to the error message.
> + return new_param_name_off;
> + }
> +
> + /* give a valid name to func_proto param as it now an enum member */
> + param->name_off = new_param_name_off;
> + }
> +
> } else if (!has_func && btf_is_func(t)) {
> /* replace FUNC with TYPEDEF */
> t->info = BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not
2025-03-31 20:09 ` [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not Timur Chernykh
@ 2025-04-02 13:05 ` Mykyta Yatsenko
0 siblings, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2025-04-02 13:05 UTC (permalink / raw)
To: Timur Chernykh, bpf
On 31/03/2025 21:09, Timur Chernykh wrote:
> Signed-off-by: Timur Chernykh <tim.cherry.co@gmail.com>
> ---
> tools/lib/bpf/features.c | 30 ++++++++++++++++
> tools/lib/bpf/libbpf.c | 62 ++++++++++++++++++++++++++++++++-
> tools/lib/bpf/libbpf_internal.h | 2 ++
> 3 files changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index 760657f5224c..dfab65f30f0c 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)
> +{
> + 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 8e1edba443dd..392779c10a73 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3113,9 +3113,10 @@ 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 +3129,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 +3265,64 @@ 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_struct(t) || btf_is_union(t) || btf_is_fwd(t) || btf_is_enum(t) || btf_is_enum64(t))) {
> + const uint16_t members_cnt = btf_vlen(t);
> +
> + /* type encoded with a kind flag */
padding seem to be broken here a little bit, I can see some spaces here.
> + if (t->info != BTF_INFO_ENC(btf_kind(t), 1, members_cnt)) {
> + continue;
> + }
> +
> + /* unset kind flag anyway */
> + t->info = BTF_INFO_ENC(btf_kind(t), 0, btf_vlen(t));
> +
> + /* structs an unions has a different bitfield processing behaviour is kind flag is set */
"if kind flag is set" ?
> + if (btf_is_struct(t) || btf_is_union(t)) {
btf_is_composite()
> + struct btf_member* members = btf_members(t);
> + struct btf_type* new_int_type = NULL;
> + int new_int_type_id;
> + __u32* new_int_type_data;
I'd go with simpler var names:
new_type, new_tid, new_type_data
> + int encoding = 0;
> + int nmember;
padding again. Did you run checkpatch.pl script on this patchset by any
chance? It should point out these type of style issues?
> +
> + for (nmember = 0; nmember < members_cnt; nmember++) {
perhaps i, j or k instead of nmember will do better here.
> + struct btf_member* member = &members[nmember];
> + const struct btf_type* member_type = btf_type_by_id(btf, member->type);
> +
> + while (btf_is_typedef(member_type)) { /* unwrap typedefs */
It looks like you should use `btf__resolve_type` here, as there are not
only typedefs, but also volatile, const types that you may need to walk
through.
Don't add {} for one line loop or if expression.
> + member_type = btf_type_by_id(btf, member_type->type);
> + }
> +
> + /* bitfields can be only int or enum values */
> + if (!(btf_is_int(member_type) || btf_is_enum(member_type))) {
> + continue;
> + }
> +
> + encoding = btf_int_encoding(member_type);
> + if (btf_is_enum(member_type) && member_type->info & 0x80000000 /* kind flag */) {
there is `btf_kflag` helper function.
> + /* enum value encodes integer signed/unsigned info in the kind flag */
> + encoding = BTF_INT_SIGNED;
> + }
> +
> + /* create new integral type with the same info */
> + snprintf(name_gen_buff, sizeof(name_gen_buff), "__int_%d_%d", i, nmember);
> + new_int_type_id = btf__add_int(btf, name_gen_buff, member_type->size, encoding);
> +
> + if (new_int_type_id < 0) {
> + pr_warn("Error adding integer type for a bitfield %d of [%d]", nmember, i);
> + return new_int_type_id;
> + }
> +
> + new_int_type = btf_type_by_id(btf, new_int_type_id);
> +
> + /* encode int in legacy way, keep offset 0 and specify bit size as set in the member */
> + new_int_type_data = (__u32*)(new_int_type + 1);
> + *new_int_type_data = BTF_INT_ENC(encoding, 0, BTF_MEMBER_BITFIELD_SIZE(member->offset));
> +
> + member->type = new_int_type_id;
> + member->offset = BTF_MEMBER_BIT_OFFSET(member->offset) /* old kernels looks only on offset */;
> + }
> + }
> }
> }
>
> 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,
> };
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-02 13:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 20:09 Improvements of BTF sanitizing for old kernels Timur Chernykh
2025-03-31 20:09 ` [PATCH 1/2] libbpf: add proto_func param name generation on sanitazing it to enum type Timur Chernykh
2025-04-02 12:41 ` Mykyta Yatsenko
2025-03-31 20:09 ` [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not Timur Chernykh
2025-04-02 13:05 ` Mykyta Yatsenko
-- strict thread matches above, loose matches on Subject: below --
2025-03-31 20:45 Improvements of BTF sanitizing for old kernels Timur Chernykh
2025-03-31 20:45 ` [PATCH 2/2] libbpf: add check if kernel supports kind flag and fix the bitfield members in union and structs if not Timur Chernykh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox