Improvements of BTF sanitizing for old kernels

Improvements of BTF sanitizing for old kernels

[Timur Chernykh]

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: 

[PATCH v2 1/2] libbpf: add proto_func param name generation

[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 = &params[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

Re: [PATCH v2 1/2] libbpf: add proto_func param name generation

[Andrii Nakryiko]

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 = &params[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 = &params[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
>
>

[PATCH v2 2/2] libbpf: add kind flag sanitizing

[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

Re: [PATCH v2 2/2] libbpf: add kind flag sanitizing

[Andrii Nakryiko]

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
>
>

Improvements of BTF sanitizing for old kernels

[Timur Chernykh]

I'm very sorry, but I found a small typo in 2nd patch, here's a
quick fix for one.

From: Timur Chernykh <tim.cherry.co@gmail.com>
To: bpf@vger.kernel.org
Subject: Improvements of BTF sanitizing for old kernels

Improvements of BTF sanitizing for old kernels

[Timur Chernykh]

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: