bpf: enforce precise retval range on program exit

Similarly to subprog/callback logic, enforce return value of BPF program
using more precise smin/smax range.

We need to adjust a bunch of tests due to a changed format of an error
message.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231202175705.885270-7-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f3d9d7de68da594e57186298193a6bf7f345f144..9411c1046268910485de206fbedd2ad06c67ea85 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -362,20 +362,23 @@ __printf(2, 3) static void verbose(void *private_data, const char *fmt, ...)
 
 static void verbose_invalid_scalar(struct bpf_verifier_env *env,
                                   struct bpf_reg_state *reg,
-                                  struct tnum *range, const char *ctx,
+                                  struct bpf_retval_range range, const char *ctx,
                                   const char *reg_name)
 {
-       char tn_buf[48];
+       bool unknown = true;
 
-       verbose(env, "At %s the register %s ", ctx, reg_name);
-       if (!tnum_is_unknown(reg->var_off)) {
-               tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
-               verbose(env, "has value %s", tn_buf);
-       } else {
-               verbose(env, "has unknown scalar value");
+       verbose(env, "At %s the register %s has", ctx, reg_name);
+       if (reg->smin_value > S64_MIN) {
+               verbose(env, " smin=%lld", reg->smin_value);
+               unknown = false;
        }
-       tnum_strn(tn_buf, sizeof(tn_buf), *range);
-       verbose(env, " should have been in %s\n", tn_buf);
+       if (reg->smax_value < S64_MAX) {
+               verbose(env, " smax=%lld", reg->smax_value);
+               unknown = false;
+       }
+       if (unknown)
+               verbose(env, " unknown scalar value");
+       verbose(env, " should have been in [%d, %d]\n", range.minval, range.maxval);
 }
 
 static bool type_may_be_null(u32 type)
@@ -9606,10 +9609,8 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
 
                /* enforce R0 return value range */
                if (!retval_range_within(callee->callback_ret_range, r0)) {
-                       struct tnum range = tnum_range(callee->callback_ret_range.minval,
-                                                      callee->callback_ret_range.maxval);
-
-                       verbose_invalid_scalar(env, r0, &range, "callback return", "R0");
+                       verbose_invalid_scalar(env, r0, callee->callback_ret_range,
+                                              "callback return", "R0");
                        return -EINVAL;
                }
                if (!calls_callback(env, callee->callsite)) {
@@ -14995,7 +14996,8 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
        struct tnum enforce_attach_type_range = tnum_unknown;
        const struct bpf_prog *prog = env->prog;
        struct bpf_reg_state *reg;
-       struct tnum range = tnum_range(0, 1), const_0 = tnum_const(0);
+       struct bpf_retval_range range = retval_range(0, 1);
+       struct bpf_retval_range const_0 = retval_range(0, 0);
        enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
        int err;
        struct bpf_func_state *frame = env->cur_state->frame[0];
@@ -15043,8 +15045,8 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
                        return -EINVAL;
                }
 
-               if (!tnum_in(const_0, reg->var_off)) {
-                       verbose_invalid_scalar(env, reg, &const_0, "async callback", reg_name);
+               if (!retval_range_within(const_0, reg)) {
+                       verbose_invalid_scalar(env, reg, const_0, "async callback", reg_name);
                        return -EINVAL;
                }
                return 0;
@@ -15070,14 +15072,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
                    env->prog->expected_attach_type == BPF_CGROUP_INET4_GETSOCKNAME ||
                    env->prog->expected_attach_type == BPF_CGROUP_INET6_GETSOCKNAME ||
                    env->prog->expected_attach_type == BPF_CGROUP_UNIX_GETSOCKNAME)
-                       range = tnum_range(1, 1);
+                       range = retval_range(1, 1);
                if (env->prog->expected_attach_type == BPF_CGROUP_INET4_BIND ||
                    env->prog->expected_attach_type == BPF_CGROUP_INET6_BIND)
-                       range = tnum_range(0, 3);
+                       range = retval_range(0, 3);
                break;
        case BPF_PROG_TYPE_CGROUP_SKB:
                if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
-                       range = tnum_range(0, 3);
+                       range = retval_range(0, 3);
                        enforce_attach_type_range = tnum_range(2, 3);
                }
                break;
@@ -15090,13 +15092,13 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
        case BPF_PROG_TYPE_RAW_TRACEPOINT:
                if (!env->prog->aux->attach_btf_id)
                        return 0;
-               range = tnum_const(0);
+               range = retval_range(0, 0);
                break;
        case BPF_PROG_TYPE_TRACING:
                switch (env->prog->expected_attach_type) {
                case BPF_TRACE_FENTRY:
                case BPF_TRACE_FEXIT:
-                       range = tnum_const(0);
+                       range = retval_range(0, 0);
                        break;
                case BPF_TRACE_RAW_TP:
                case BPF_MODIFY_RETURN:
@@ -15108,7 +15110,7 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
                }
                break;
        case BPF_PROG_TYPE_SK_LOOKUP:
-               range = tnum_range(SK_DROP, SK_PASS);
+               range = retval_range(SK_DROP, SK_PASS);
                break;
 
        case BPF_PROG_TYPE_LSM:
@@ -15122,12 +15124,12 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
                        /* Make sure programs that attach to void
                         * hooks don't try to modify return value.
                         */
-                       range = tnum_range(1, 1);
+                       range = retval_range(1, 1);
                }
                break;
 
        case BPF_PROG_TYPE_NETFILTER:
-               range = tnum_range(NF_DROP, NF_ACCEPT);
+               range = retval_range(NF_DROP, NF_ACCEPT);
                break;
        case BPF_PROG_TYPE_EXT:
                /* freplace program can return anything as its return value
@@ -15143,8 +15145,8 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
                return -EINVAL;
        }
 
-       if (!tnum_in(range, reg->var_off)) {
-               verbose_invalid_scalar(env, reg, &range, "program exit", reg_name);
+       if (!retval_range_within(range, reg)) {
+               verbose_invalid_scalar(env, reg, range, "program exit", reg_name);
                if (prog->expected_attach_type == BPF_LSM_CGROUP &&
                    prog_type == BPF_PROG_TYPE_LSM &&
                    !prog->aux->attach_func_proto->type)

diff --git a/tools/testing/selftests/bpf/progs/exceptions_assert.c b/tools/testing/selftests/bpf/progs/exceptions_assert.c
index 575e7dd719c4b2fce3195fc840a9e577ec2fd533..0ef81040da59607acb71d729616398308178c79a 100644 (file)
--- a/tools/testing/selftests/bpf/progs/exceptions_assert.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_assert.c
@@ -125,7 +125,7 @@ int check_assert_generic(struct __sk_buff *ctx)
 }
 
 SEC("?fentry/bpf_check")
-__failure __msg("At program exit the register R1 has value (0x40; 0x0)")
+__failure __msg("At program exit the register R1 has smin=64 smax=64")
 int check_assert_with_return(void *ctx)
 {
        bpf_assert_with(!ctx, 64);

diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index 81ead7512ba25ec2929beb9435b122888cf97af0..9cceb652114335e9ab735fc63e96e5ffb66393f6 100644 (file)
--- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c
@@ -308,7 +308,7 @@ int reject_set_exception_cb_bad_ret1(void *ctx)
 }
 
 SEC("?fentry/bpf_check")
-__failure __msg("At program exit the register R1 has value (0x40; 0x0) should")
+__failure __msg("At program exit the register R1 has smin=64 smax=64 should")
 int reject_set_exception_cb_bad_ret2(void *ctx)
 {
        bpf_throw(64);

diff --git a/tools/testing/selftests/bpf/progs/test_global_func15.c b/tools/testing/selftests/bpf/progs/test_global_func15.c
index b512d6a6c75e54e700860ac0f4098a1ea7e7a24e..f80207480e8ad0ab348d5b347966f52e6ebb508f 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_global_func15.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func15.c
@@ -13,7 +13,7 @@ __noinline int foo(unsigned int *v)
 }
 
 SEC("cgroup_skb/ingress")
-__failure __msg("At program exit the register R0 has value")
+__failure __msg("At program exit the register R0 has ")
 int global_func15(struct __sk_buff *skb)
 {
        unsigned int v = 1;

diff --git a/tools/testing/selftests/bpf/progs/timer_failure.c b/tools/testing/selftests/bpf/progs/timer_failure.c
index 226d33b5a05c800acc9d40b1a1fecd1a146c4a23..9000da1e2120ab18c31886f485487dc2a8cf70b9 100644 (file)
--- a/tools/testing/selftests/bpf/progs/timer_failure.c
+++ b/tools/testing/selftests/bpf/progs/timer_failure.c
@@ -30,7 +30,7 @@ static int timer_cb_ret1(void *map, int *key, struct bpf_timer *timer)
 }
 
 SEC("fentry/bpf_fentry_test1")
-__failure __msg("should have been in (0x0; 0x0)")
+__failure __msg("should have been in [0, 0]")
 int BPF_PROG2(test_ret_1, int, a)
 {
        int key = 0;

diff --git a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
index 03ee946c6bf76a36738c6e068ee8cc9b448916eb..11ab25c42c360ad0197a70d44835e6f77d926204 100644 (file)
--- a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
+++ b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
@@ -184,7 +184,7 @@ invalid_drain_callback_return(struct bpf_dynptr *dynptr, void *context)
  * not be able to write to that pointer.
  */
 SEC("?raw_tp")
-__failure __msg("At callback return the register R0 has value")
+__failure __msg("At callback return the register R0 has ")
 int user_ringbuf_callback_invalid_return(void *ctx)
 {
        bpf_user_ringbuf_drain(&user_ringbuf, invalid_drain_callback_return, NULL, 0);

diff --git a/tools/testing/selftests/bpf/progs/verifier_cgroup_inv_retcode.c b/tools/testing/selftests/bpf/progs/verifier_cgroup_inv_retcode.c
index d6c4a7f3f7900e2db204e40fa55e01b06e827059..6e0f349f8f15fa1aba220a3ef7d743a7cc3eab69 100644 (file)
--- a/tools/testing/selftests/bpf/progs/verifier_cgroup_inv_retcode.c
+++ b/tools/testing/selftests/bpf/progs/verifier_cgroup_inv_retcode.c
@@ -7,7 +7,7 @@
 
 SEC("cgroup/sock")
 __description("bpf_exit with invalid return code. test1")
-__failure __msg("R0 has value (0x0; 0xffffffff)")
+__failure __msg("smin=0 smax=4294967295 should have been in [0, 1]")
 __naked void with_invalid_return_code_test1(void)
 {
        asm volatile ("                                 \
@@ -30,7 +30,7 @@ __naked void with_invalid_return_code_test2(void)
 
 SEC("cgroup/sock")
 __description("bpf_exit with invalid return code. test3")
-__failure __msg("R0 has value (0x0; 0x3)")
+__failure __msg("smin=0 smax=3 should have been in [0, 1]")
 __naked void with_invalid_return_code_test3(void)
 {
        asm volatile ("                                 \
@@ -53,7 +53,7 @@ __naked void with_invalid_return_code_test4(void)
 
 SEC("cgroup/sock")
 __description("bpf_exit with invalid return code. test5")
-__failure __msg("R0 has value (0x2; 0x0)")
+__failure __msg("smin=2 smax=2 should have been in [0, 1]")
 __naked void with_invalid_return_code_test5(void)
 {
        asm volatile ("                                 \
@@ -75,7 +75,7 @@ __naked void with_invalid_return_code_test6(void)
 
 SEC("cgroup/sock")
 __description("bpf_exit with invalid return code. test7")
-__failure __msg("R0 has unknown scalar value")
+__failure __msg("R0 has unknown scalar value should have been in [0, 1]")
 __naked void with_invalid_return_code_test7(void)
 {
        asm volatile ("                                 \

diff --git a/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c b/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c
index 353ae6da00e17e595f85cb113fa891963a7b5882..e1ffa5d32ff0646e177b0f9724fd6aadbafc360a 100644 (file)
--- a/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c
+++ b/tools/testing/selftests/bpf/progs/verifier_netfilter_retcode.c
@@ -39,7 +39,7 @@ __naked void with_valid_return_code_test3(void)
 
 SEC("netfilter")
 __description("bpf_exit with invalid return code. test4")
-__failure __msg("R0 has value (0x2; 0x0)")
+__failure __msg("R0 has smin=2 smax=2 should have been in [0, 1]")
 __naked void with_invalid_return_code_test4(void)
 {
        asm volatile ("                                 \

diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
index d41d2a8bb97eb3a41ba178530a81bb01b0d25dff..0dfe3f8b69acff7137dd1b720495e037a28fc4c9 100644 (file)
--- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
@@ -148,7 +148,7 @@ __msg("mark_precise: frame1: regs=r0 stack= before 11: (b7) r0 = 0")
 __msg("from 10 to 12: frame1: R0=scalar(smin=umin=1001")
 /* check that branch code path marks r0 as precise, before failing */
 __msg("mark_precise: frame1: regs=r0 stack= before 9: (85) call bpf_get_prandom_u32#7")
-__msg("At callback return the register R0 has value (0x0; 0x7fffffffffffffff) should have been in (0x0; 0x1)")
+__msg("At callback return the register R0 has smin=1001 should have been in [0, 1]")
 __naked int callback_precise_return_fail(void)
 {
        asm volatile (