History

bpf: Small BPF verifier log improvements

In particular these include: 1) Remove output of inv for scalars in print_verifier_state 2) Replace inv with scalar in verifier error messages 3) Remove _value suffixes for umin/umax/s32_min/etc (except map_value) 4) Remove output of id=0 5) Remove output of ref_obj_id=0

Mykola Lysenko committed 4 years ago

Commit 7df50721 parent 41332d620 files · +203 −197Browse files

Showing the changes to kernel (1 of 20 files). Show all

Unified · Split

kernel/bpf/verifier.c+35−29
@@ -539,7 +539,7 @@ static const char *reg_type_str(struct bpf_verifier_env *env, · 538 unmodified lines
539539 char postfix[16] = {0}, prefix[32] = {0};
540540 static const char * const str[] = {
541541 [NOT_INIT] = "?",
542− [SCALAR_VALUE] = "inv",
542+ [SCALAR_VALUE] = "scalar",
543543 [PTR_TO_CTX] = "ctx",
544544 [CONST_PTR_TO_MAP] = "map_ptr",
545545 [PTR_TO_MAP_VALUE] = "map_value",
@@ -685,74 +685,80 @@ static void print_verifier_state(struct bpf_verifier_env *env, · 139 unmodified lines
685685 continue;
686686 verbose(env, " R%d", i);
687687 print_liveness(env, reg->live);
688− verbose(env, "=%s", reg_type_str(env, t));
688+ verbose(env, "=");
689689 if (t == SCALAR_VALUE && reg->precise)
690690 verbose(env, "P");
691691 if ((t == SCALAR_VALUE || t == PTR_TO_STACK) &&
692692 tnum_is_const(reg->var_off)) {
693693 /* reg->off should be 0 for SCALAR_VALUE */
694+ verbose(env, "%s", t == SCALAR_VALUE ? "" : reg_type_str(env, t));
694695 verbose(env, "%lld", reg->var_off.value + reg->off);
695696 } else {
697+ const char *sep = "";
698+
699+ verbose(env, "%s", reg_type_str(env, t));
696700 if (base_type(t) == PTR_TO_BTF_ID ||
697701 base_type(t) == PTR_TO_PERCPU_BTF_ID)
698702 verbose(env, "%s", kernel_type_name(reg->btf, reg->btf_id));
699− verbose(env, "(id=%d", reg->id);
700− if (reg_type_may_be_refcounted_or_null(t))
701− verbose(env, ",ref_obj_id=%d", reg->ref_obj_id);
703+ verbose(env, "(");
704+/*
705+ * _a stands for append, was shortened to avoid multiline statements below.
706+ * This macro is used to output a comma separated list of attributes.
707+ */
708+#define verbose_a(fmt, ...) ({ verbose(env, "%s" fmt, sep, __VA_ARGS__); sep = ","; })
709+
710+ if (reg->id)
711+ verbose_a("id=%d", reg->id);
712+ if (reg_type_may_be_refcounted_or_null(t) && reg->ref_obj_id)
713+ verbose_a("ref_obj_id=%d", reg->ref_obj_id);
702714 if (t != SCALAR_VALUE)
703− verbose(env, ",off=%d", reg->off);
715+ verbose_a("off=%d", reg->off);
704716 if (type_is_pkt_pointer(t))
705− verbose(env, ",r=%d", reg->range);
717+ verbose_a("r=%d", reg->range);
706718 else if (base_type(t) == CONST_PTR_TO_MAP ||
707719 base_type(t) == PTR_TO_MAP_KEY ||
708720 base_type(t) == PTR_TO_MAP_VALUE)
709− verbose(env, ",ks=%d,vs=%d",
710− reg->map_ptr->key_size,
711− reg->map_ptr->value_size);
721+ verbose_a("ks=%d,vs=%d",
722+ reg->map_ptr->key_size,
723+ reg->map_ptr->value_size);
712724 if (tnum_is_const(reg->var_off)) {
713725 /* Typically an immediate SCALAR_VALUE, but
714726 * could be a pointer whose offset is too big
715727 * for reg->off
716728 */
717− verbose(env, ",imm=%llx", reg->var_off.value);
729+ verbose_a("imm=%llx", reg->var_off.value);
718730 } else {
719731 if (reg->smin_value != reg->umin_value &&
720732 reg->smin_value != S64_MIN)
721− verbose(env, ",smin_value=%lld",
722− (long long)reg->smin_value);
733+ verbose_a("smin=%lld", (long long)reg->smin_value);
723734 if (reg->smax_value != reg->umax_value &&
724735 reg->smax_value != S64_MAX)
725− verbose(env, ",smax_value=%lld",
726− (long long)reg->smax_value);
736+ verbose_a("smax=%lld", (long long)reg->smax_value);
727737 if (reg->umin_value != 0)
728− verbose(env, ",umin_value=%llu",
729− (unsigned long long)reg->umin_value);
738+ verbose_a("umin=%llu", (unsigned long long)reg->umin_value);
730739 if (reg->umax_value != U64_MAX)
731− verbose(env, ",umax_value=%llu",
732− (unsigned long long)reg->umax_value);
740+ verbose_a("umax=%llu", (unsigned long long)reg->umax_value);
733741 if (!tnum_is_unknown(reg->var_off)) {
734742 char tn_buf[48];
735743
736744 tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
737− verbose(env, ",var_off=%s", tn_buf);
745+ verbose_a("var_off=%s", tn_buf);
738746 }
739747 if (reg->s32_min_value != reg->smin_value &&
740748 reg->s32_min_value != S32_MIN)
741− verbose(env, ",s32_min_value=%d",
742− (int)(reg->s32_min_value));
749+ verbose_a("s32_min=%d", (int)(reg->s32_min_value));
743750 if (reg->s32_max_value != reg->smax_value &&
744751 reg->s32_max_value != S32_MAX)
745− verbose(env, ",s32_max_value=%d",
746− (int)(reg->s32_max_value));
752+ verbose_a("s32_max=%d", (int)(reg->s32_max_value));
747753 if (reg->u32_min_value != reg->umin_value &&
748754 reg->u32_min_value != U32_MIN)
749− verbose(env, ",u32_min_value=%d",
750− (int)(reg->u32_min_value));
755+ verbose_a("u32_min=%d", (int)(reg->u32_min_value));
751756 if (reg->u32_max_value != reg->umax_value &&
752757 reg->u32_max_value != U32_MAX)
753− verbose(env, ",u32_max_value=%d",
754− (int)(reg->u32_max_value));
758+ verbose_a("u32_max=%d", (int)(reg->u32_max_value));
755759 }
760+#undef verbose_a
761+
756762 verbose(env, ")");
757763 }
758764 }
@@ -777,7 +783,7 @@ static void print_verifier_state(struct bpf_verifier_env *env, · 18 unmodified lines
777783 if (is_spilled_reg(&state->stack[i])) {
778784 reg = &state->stack[i].spilled_ptr;
779785 t = reg->type;
780− verbose(env, "=%s", reg_type_str(env, t));
786+ verbose(env, "=%s", t == SCALAR_VALUE ? "" : reg_type_str(env, t));
781787 if (t == SCALAR_VALUE && reg->precise)
782788 verbose(env, "P");
783789 if (t == SCALAR_VALUE && tnum_is_const(reg->var_off))