* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.