diff mbox series

[RFC,bpf-next,4/8] btf: support kernel parsing of BTF with metadata, use it to parse BTF with unknown kinds

Message ID 20230531201936.1992188-5-alan.maguire@oracle.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series bpf: support BTF kind metadata to separate | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 fail Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 fail Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 success Logs for veristat
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 14 this patch: 14
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 14 this patch: 14
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns WARNING: line length of 98 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Alan Maguire May 31, 2023, 8:19 p.m. UTC 
Validate metadata if present, and use it to parse BTF with
unrecognized kinds. Reject BTF that contains a type
of a kind that is not optional.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 kernel/bpf/btf.c | 102 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 85 insertions(+), 17 deletions(-)

Comments

Eduard Zingerman June 7, 2023, 7:51 p.m. UTC | #1 
On Wed, 2023-05-31 at 21:19 +0100, Alan Maguire wrote:
> Validate metadata if present, and use it to parse BTF with
> unrecognized kinds. Reject BTF that contains a type
> of a kind that is not optional.
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  kernel/bpf/btf.c | 102 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 85 insertions(+), 17 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index bd2cac057928..67f42d9ce099 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -257,6 +257,7 @@ struct btf {
>  	struct btf_kfunc_set_tab *kfunc_set_tab;
>  	struct btf_id_dtor_kfunc_tab *dtor_kfunc_tab;
>  	struct btf_struct_metas *struct_meta_tab;
> +	struct btf_metadata *meta_data;
>  
>  	/* split BTF support */
>  	struct btf *base_btf;
> @@ -4965,22 +4966,41 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
>  		return -EINVAL;
>  	}
>  
> -	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
> -	    BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
> -		btf_verifier_log(env, "[%u] Invalid kind:%u",
> -				 env->log_type_id, BTF_INFO_KIND(t->info));
> -		return -EINVAL;
> -	}
> -
>  	if (!btf_name_offset_valid(env->btf, t->name_off)) {
>  		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
>  				 env->log_type_id, t->name_off);
>  		return -EINVAL;
>  	}
>  
> -	var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> -	if (var_meta_size < 0)
> -		return var_meta_size;
> +	if (BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
> +		btf_verifier_log(env, "[%u] Invalid kind:%u",
> +				 env->log_type_id, BTF_INFO_KIND(t->info));
> +		return -EINVAL;
> +	}
> +
> +	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->meta_data &&
> +	    BTF_INFO_KIND(t->info) < env->btf->meta_data->kind_meta_cnt) {
> +		struct btf_kind_meta *m = &env->btf->meta_data->kind_meta[BTF_INFO_KIND(t->info)];
> +
> +		if (!(m->flags & BTF_KIND_META_OPTIONAL)) {
> +			btf_verifier_log(env, "[%u] unknown but required kind '%s'(%u)",
> +					 env->log_type_id,
> +					 btf_name_by_offset(env->btf, m->name_off),
> +					 BTF_INFO_KIND(t->info));
> +			return -EINVAL;
> +		}
> +		var_meta_size = sizeof(struct btf_type);
> +		var_meta_size += m->info_sz + (btf_type_vlen(t) * m->elem_sz);
> +	} else {
> +		if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX) {
> +			btf_verifier_log(env, "[%u] Invalid kind:%u",
> +					 env->log_type_id, BTF_INFO_KIND(t->info));
> +			return -EINVAL;
> +		}
> +		var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> +		if (var_meta_size < 0)
> +			return var_meta_size;
> +	}
>  
>  	meta_left -= var_meta_size;
>  
> @@ -5155,7 +5175,8 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>  	start = btf->nohdr_data + hdr->str_off;
>  	end = start + hdr->str_len;
>  
> -	if (end != btf->data + btf->data_size) {
> +	if (hdr->hdr_len < sizeof(struct btf_header) &&
> +	    end != btf->data + btf->data_size) {
>  		btf_verifier_log(env, "String section is not at the end");
>  		return -EINVAL;
>  	}
> @@ -5176,9 +5197,47 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>  	return 0;
>  }
>  
> +static int btf_parse_meta_sec(struct btf_verifier_env *env)
> +{
> +	const struct btf_header *hdr = &env->btf->hdr;
> +	struct btf *btf = env->btf;
> +	void *start, *end;
> +
> +	if (hdr->hdr_len < sizeof(struct btf_header) ||
> +	    hdr->meta_header.meta_len == 0)
> +		return 0;
> +
> +	/* Meta section must align to 8 bytes */
> +	if (hdr->meta_header.meta_off & (sizeof(u64) - 1)) {
> +		btf_verifier_log(env, "Unaligned meta_off");
> +		return -EINVAL;
> +	}
> +	start = btf->nohdr_data + hdr->meta_header.meta_off;
> +	end = start + hdr->meta_header.meta_len;
> +
> +	if (hdr->meta_header.meta_len < sizeof(struct btf_meta_header)) {
> +		btf_verifier_log(env, "Metadata section is too small");
> +		return -EINVAL;
> +	}
> +	if (end != btf->data + btf->data_size) {
> +		btf_verifier_log(env, "Metadata section is not at the end");
> +		return -EINVAL;
> +	}
> +	btf->meta_data = start;
> +
> +	if (hdr->meta_header.meta_len != sizeof(struct btf_metadata) +
> +					(btf->meta_data->kind_meta_cnt *
> +					 sizeof(struct btf_kind_meta))) {
> +		btf_verifier_log(env, "Metadata section size mismatch");
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>  static const size_t btf_sec_info_offset[] = {
>  	offsetof(struct btf_header, type_off),
>  	offsetof(struct btf_header, str_off),
> +	offsetof(struct btf_header, meta_header.meta_off),
>  };
>  
>  static int btf_sec_info_cmp(const void *a, const void *b)
> @@ -5193,15 +5252,19 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>  			      u32 btf_data_size)
>  {
>  	struct btf_sec_info secs[ARRAY_SIZE(btf_sec_info_offset)];
> -	u32 total, expected_total, i;
> +	u32 nr_secs = ARRAY_SIZE(btf_sec_info_offset);
> +	u32 total, expected_total, gap, i;
>  	const struct btf_header *hdr;
>  	const struct btf *btf;
>  
>  	btf = env->btf;
>  	hdr = &btf->hdr;
>  
> +	if (hdr->hdr_len < sizeof(struct btf_header))
> +		nr_secs--;
> +
>  	/* Populate the secs from hdr */
> -	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++)
> +	for (i = 0; i < nr_secs; i++)
>  		secs[i] = *(struct btf_sec_info *)((void *)hdr +
>  						   btf_sec_info_offset[i]);
>  
> @@ -5216,9 +5279,10 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>  			btf_verifier_log(env, "Invalid section offset");
>  			return -EINVAL;
>  		}
> -		if (total < secs[i].off) {
> -			/* gap */
> -			btf_verifier_log(env, "Unsupported section found");
> +		gap = secs[i].off - total;
> +		if (gap >= 8) {
> +			/* gap larger than alignment gap */
> +			btf_verifier_log(env, "Unsupported section gap found");

I have two `prog_tests` tests failing with this patch applied:
* btf/btf_header test. Gap between hdr and type
  Here expected string should be updated:

--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c
@@ -1579,7 +1579,7 @@ static struct btf_raw_test raw_tests[] = {
        .max_entries = 4,
        .btf_load_err = true,
        .type_off_delta = 4,
-       .err_str = "Unsupported section found",
+       .err_str = "Unsupported section gap found",
 },

* btf/btf_header test. Overlap between type and str
  This test expects error string "Section overlap found",
  but instead "Section overlap found" is printed.
  This happens with the following values of local variables:
    
    total=20, secs[2].off=16, gap=-4

  (`gap` is printed as signed using %d).

>  			return -EINVAL;
>  		}
>  		if (total > secs[i].off) {
> @@ -5230,7 +5294,7 @@ static int btf_check_sec_info(struct btf_verifier_env *env,
>  					 "Total section length too long");
>  			return -EINVAL;
>  		}
> -		total += secs[i].len;
> +		total += secs[i].len + gap;
>  	}
>  
>  	/* There is data other than hdr and known sections */
> @@ -5530,6 +5594,10 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
>  	if (err)
>  		goto errout;
>  
> +	err = btf_parse_meta_sec(env);
> +	if (err)
> +		goto errout;
> +
>  	err = btf_parse_type_sec(env);
>  	if (err)
>  		goto errout;
diff mbox series

Patch

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index bd2cac057928..67f42d9ce099 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -257,6 +257,7 @@  struct btf {
 	struct btf_kfunc_set_tab *kfunc_set_tab;
 	struct btf_id_dtor_kfunc_tab *dtor_kfunc_tab;
 	struct btf_struct_metas *struct_meta_tab;
+	struct btf_metadata *meta_data;
 
 	/* split BTF support */
 	struct btf *base_btf;
@@ -4965,22 +4966,41 @@  static s32 btf_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
-	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
-	    BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
-		btf_verifier_log(env, "[%u] Invalid kind:%u",
-				 env->log_type_id, BTF_INFO_KIND(t->info));
-		return -EINVAL;
-	}
-
 	if (!btf_name_offset_valid(env->btf, t->name_off)) {
 		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
 				 env->log_type_id, t->name_off);
 		return -EINVAL;
 	}
 
-	var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
-	if (var_meta_size < 0)
-		return var_meta_size;
+	if (BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
+		btf_verifier_log(env, "[%u] Invalid kind:%u",
+				 env->log_type_id, BTF_INFO_KIND(t->info));
+		return -EINVAL;
+	}
+
+	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->meta_data &&
+	    BTF_INFO_KIND(t->info) < env->btf->meta_data->kind_meta_cnt) {
+		struct btf_kind_meta *m = &env->btf->meta_data->kind_meta[BTF_INFO_KIND(t->info)];
+
+		if (!(m->flags & BTF_KIND_META_OPTIONAL)) {
+			btf_verifier_log(env, "[%u] unknown but required kind '%s'(%u)",
+					 env->log_type_id,
+					 btf_name_by_offset(env->btf, m->name_off),
+					 BTF_INFO_KIND(t->info));
+			return -EINVAL;
+		}
+		var_meta_size = sizeof(struct btf_type);
+		var_meta_size += m->info_sz + (btf_type_vlen(t) * m->elem_sz);
+	} else {
+		if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX) {
+			btf_verifier_log(env, "[%u] Invalid kind:%u",
+					 env->log_type_id, BTF_INFO_KIND(t->info));
+			return -EINVAL;
+		}
+		var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
+		if (var_meta_size < 0)
+			return var_meta_size;
+	}
 
 	meta_left -= var_meta_size;
 
@@ -5155,7 +5175,8 @@  static int btf_parse_str_sec(struct btf_verifier_env *env)
 	start = btf->nohdr_data + hdr->str_off;
 	end = start + hdr->str_len;
 
-	if (end != btf->data + btf->data_size) {
+	if (hdr->hdr_len < sizeof(struct btf_header) &&
+	    end != btf->data + btf->data_size) {
 		btf_verifier_log(env, "String section is not at the end");
 		return -EINVAL;
 	}
@@ -5176,9 +5197,47 @@  static int btf_parse_str_sec(struct btf_verifier_env *env)
 	return 0;
 }
 
+static int btf_parse_meta_sec(struct btf_verifier_env *env)
+{
+	const struct btf_header *hdr = &env->btf->hdr;
+	struct btf *btf = env->btf;
+	void *start, *end;
+
+	if (hdr->hdr_len < sizeof(struct btf_header) ||
+	    hdr->meta_header.meta_len == 0)
+		return 0;
+
+	/* Meta section must align to 8 bytes */
+	if (hdr->meta_header.meta_off & (sizeof(u64) - 1)) {
+		btf_verifier_log(env, "Unaligned meta_off");
+		return -EINVAL;
+	}
+	start = btf->nohdr_data + hdr->meta_header.meta_off;
+	end = start + hdr->meta_header.meta_len;
+
+	if (hdr->meta_header.meta_len < sizeof(struct btf_meta_header)) {
+		btf_verifier_log(env, "Metadata section is too small");
+		return -EINVAL;
+	}
+	if (end != btf->data + btf->data_size) {
+		btf_verifier_log(env, "Metadata section is not at the end");
+		return -EINVAL;
+	}
+	btf->meta_data = start;
+
+	if (hdr->meta_header.meta_len != sizeof(struct btf_metadata) +
+					(btf->meta_data->kind_meta_cnt *
+					 sizeof(struct btf_kind_meta))) {
+		btf_verifier_log(env, "Metadata section size mismatch");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static const size_t btf_sec_info_offset[] = {
 	offsetof(struct btf_header, type_off),
 	offsetof(struct btf_header, str_off),
+	offsetof(struct btf_header, meta_header.meta_off),
 };
 
 static int btf_sec_info_cmp(const void *a, const void *b)
@@ -5193,15 +5252,19 @@  static int btf_check_sec_info(struct btf_verifier_env *env,
 			      u32 btf_data_size)
 {
 	struct btf_sec_info secs[ARRAY_SIZE(btf_sec_info_offset)];
-	u32 total, expected_total, i;
+	u32 nr_secs = ARRAY_SIZE(btf_sec_info_offset);
+	u32 total, expected_total, gap, i;
 	const struct btf_header *hdr;
 	const struct btf *btf;
 
 	btf = env->btf;
 	hdr = &btf->hdr;
 
+	if (hdr->hdr_len < sizeof(struct btf_header))
+		nr_secs--;
+
 	/* Populate the secs from hdr */
-	for (i = 0; i < ARRAY_SIZE(btf_sec_info_offset); i++)
+	for (i = 0; i < nr_secs; i++)
 		secs[i] = *(struct btf_sec_info *)((void *)hdr +
 						   btf_sec_info_offset[i]);
 
@@ -5216,9 +5279,10 @@  static int btf_check_sec_info(struct btf_verifier_env *env,
 			btf_verifier_log(env, "Invalid section offset");
 			return -EINVAL;
 		}
-		if (total < secs[i].off) {
-			/* gap */
-			btf_verifier_log(env, "Unsupported section found");
+		gap = secs[i].off - total;
+		if (gap >= 8) {
+			/* gap larger than alignment gap */
+			btf_verifier_log(env, "Unsupported section gap found");
 			return -EINVAL;
 		}
 		if (total > secs[i].off) {
@@ -5230,7 +5294,7 @@  static int btf_check_sec_info(struct btf_verifier_env *env,
 					 "Total section length too long");
 			return -EINVAL;
 		}
-		total += secs[i].len;
+		total += secs[i].len + gap;
 	}
 
 	/* There is data other than hdr and known sections */
@@ -5530,6 +5594,10 @@  static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
 	if (err)
 		goto errout;
 
+	err = btf_parse_meta_sec(env);
+	if (err)
+		goto errout;
+
 	err = btf_parse_type_sec(env);
 	if (err)
 		goto errout;