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@xxxxxxxxxx>
> ---
> 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;
