selftests/bpf: track string payload offset as scalar in strobemeta

commit 87eb0152bcc102ecbda866978f4e54db5a3be1ef upstream.

This change prepares strobemeta for update in callbacks verification
logic. To allow bpf_loop() verification converge when multiple
callback iterations are considered:
- track offset inside strobemeta_payload->payload directly as scalar
  value;
- at each iteration make sure that remaining
  strobemeta_payload->payload capacity is sufficient for execution of
  read_{map,str}_var functions;
- make sure that offset is tracked as unbound scalar between
  iterations, otherwise verifier won't be able infer that bpf_loop
  callback reaches identical states.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/r/20231121020701.26440-3-eddyz87@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/tools/testing/selftests/bpf/progs/strobemeta.h b/tools/testing/selftests/bpf/progs/strobemeta.h
index e02cfd3..40df2cc 100644 (file)
--- a/tools/testing/selftests/bpf/progs/strobemeta.h
+++ b/tools/testing/selftests/bpf/progs/strobemeta.h
@@ -24,9 +24,11 @@ struct task_struct {};
 #define STACK_TABLE_EPOCH_SHIFT 20
 #define STROBE_MAX_STR_LEN 1
 #define STROBE_MAX_CFGS 32
+#define READ_MAP_VAR_PAYLOAD_CAP                                       \
+       ((1 + STROBE_MAX_MAP_ENTRIES * 2) * STROBE_MAX_STR_LEN)
 #define STROBE_MAX_PAYLOAD                                             \
        (STROBE_MAX_STRS * STROBE_MAX_STR_LEN +                         \
-       STROBE_MAX_MAPS * (1 + STROBE_MAX_MAP_ENTRIES * 2) * STROBE_MAX_STR_LEN)
+        STROBE_MAX_MAPS * READ_MAP_VAR_PAYLOAD_CAP)
 
 struct strobe_value_header {
        /*
@@ -355,7 +357,7 @@ static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,
                                             size_t idx, void *tls_base,
                                             struct strobe_value_generic *value,
                                             struct strobemeta_payload *data,
-                                            void *payload)
+                                            size_t off)
 {
        void *location;
        uint64_t len;
@@ -366,7 +368,7 @@ static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,
                return 0;
 
        bpf_probe_read_user(value, sizeof(struct strobe_value_generic), location);
-       len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN, value->ptr);
+       len = bpf_probe_read_user_str(&data->payload[off], STROBE_MAX_STR_LEN, value->ptr);
        /*
         * if bpf_probe_read_user_str returns error (<0), due to casting to
         * unsinged int, it will become big number, so next check is
@@ -378,14 +380,14 @@ static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,
                return 0;
 
        data->str_lens[idx] = len;
-       return len;
+       return off + len;
 }
 
-static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
-                                         size_t idx, void *tls_base,
-                                         struct strobe_value_generic *value,
-                                         struct strobemeta_payload *data,
-                                         void *payload)
+static __always_inline uint64_t read_map_var(struct strobemeta_cfg *cfg,
+                                            size_t idx, void *tls_base,
+                                            struct strobe_value_generic *value,
+                                            struct strobemeta_payload *data,
+                                            size_t off)
 {
        struct strobe_map_descr* descr = &data->map_descrs[idx];
        struct strobe_map_raw map;
@@ -397,11 +399,11 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
 
        location = calc_location(&cfg->map_locs[idx], tls_base);
        if (!location)
-               return payload;
+               return off;
 
        bpf_probe_read_user(value, sizeof(struct strobe_value_generic), location);
        if (bpf_probe_read_user(&map, sizeof(struct strobe_map_raw), value->ptr))
-               return payload;
+               return off;
 
        descr->id = map.id;
        descr->cnt = map.cnt;
@@ -410,10 +412,10 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
                data->req_meta_valid = 1;
        }
 
-       len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN, map.tag);
+       len = bpf_probe_read_user_str(&data->payload[off], STROBE_MAX_STR_LEN, map.tag);
        if (len <= STROBE_MAX_STR_LEN) {
                descr->tag_len = len;
-               payload += len;
+               off += len;
        }
 
 #ifdef NO_UNROLL
@@ -426,22 +428,22 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
                        break;
 
                descr->key_lens[i] = 0;
-               len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN,
+               len = bpf_probe_read_user_str(&data->payload[off], STROBE_MAX_STR_LEN,
                                              map.entries[i].key);
                if (len <= STROBE_MAX_STR_LEN) {
                        descr->key_lens[i] = len;
-                       payload += len;
+                       off += len;
                }
                descr->val_lens[i] = 0;
-               len = bpf_probe_read_user_str(payload, STROBE_MAX_STR_LEN,
+               len = bpf_probe_read_user_str(&data->payload[off], STROBE_MAX_STR_LEN,
                                              map.entries[i].val);
                if (len <= STROBE_MAX_STR_LEN) {
                        descr->val_lens[i] = len;
-                       payload += len;
+                       off += len;
                }
        }
 
-       return payload;
+       return off;
 }
 
 #ifdef USE_BPF_LOOP
@@ -455,14 +457,20 @@ struct read_var_ctx {
        struct strobemeta_payload *data;
        void *tls_base;
        struct strobemeta_cfg *cfg;
-       void *payload;
+       size_t payload_off;
        /* value gets mutated */
        struct strobe_value_generic *value;
        enum read_type type;
 };
 
-static int read_var_callback(__u32 index, struct read_var_ctx *ctx)
+static int read_var_callback(__u64 index, struct read_var_ctx *ctx)
 {
+       /* lose precision info for ctx->payload_off, verifier won't track
+        * double xor, barrier_var() is needed to force clang keep both xors.
+        */
+       ctx->payload_off ^= index;
+       barrier_var(ctx->payload_off);
+       ctx->payload_off ^= index;
        switch (ctx->type) {
        case READ_INT_VAR:
                if (index >= STROBE_MAX_INTS)
@@ -472,14 +480,18 @@ static int read_var_callback(__u32 index, struct read_var_ctx *ctx)
        case READ_MAP_VAR:
                if (index >= STROBE_MAX_MAPS)
                        return 1;
-               ctx->payload = read_map_var(ctx->cfg, index, ctx->tls_base,
-                                           ctx->value, ctx->data, ctx->payload);
+               if (ctx->payload_off > sizeof(ctx->data->payload) - READ_MAP_VAR_PAYLOAD_CAP)
+                       return 1;
+               ctx->payload_off = read_map_var(ctx->cfg, index, ctx->tls_base,
+                                               ctx->value, ctx->data, ctx->payload_off);
                break;
        case READ_STR_VAR:
                if (index >= STROBE_MAX_STRS)
                        return 1;
-               ctx->payload += read_str_var(ctx->cfg, index, ctx->tls_base,
-                                            ctx->value, ctx->data, ctx->payload);
+               if (ctx->payload_off > sizeof(ctx->data->payload) - STROBE_MAX_STR_LEN)
+                       return 1;
+               ctx->payload_off = read_str_var(ctx->cfg, index, ctx->tls_base,
+                                               ctx->value, ctx->data, ctx->payload_off);
                break;
        }
        return 0;
@@ -501,7 +513,8 @@ static void *read_strobe_meta(struct task_struct *task,
        pid_t pid = bpf_get_current_pid_tgid() >> 32;
        struct strobe_value_generic value = {0};
        struct strobemeta_cfg *cfg;
-       void *tls_base, *payload;
+       size_t payload_off;
+       void *tls_base;
 
        cfg = bpf_map_lookup_elem(&strobemeta_cfgs, &pid);
        if (!cfg)
@@ -509,7 +522,7 @@ static void *read_strobe_meta(struct task_struct *task,
 
        data->int_vals_set_mask = 0;
        data->req_meta_valid = 0;
-       payload = data->payload;
+       payload_off = 0;
        /*
         * we don't have struct task_struct definition, it should be:
         * tls_base = (void *)task->thread.fsbase;
@@ -522,7 +535,7 @@ static void *read_strobe_meta(struct task_struct *task,
                .tls_base = tls_base,
                .value = &value,
                .data = data,
-               .payload = payload,
+               .payload_off = 0,
        };
        int err;
 
@@ -540,6 +553,11 @@ static void *read_strobe_meta(struct task_struct *task,
        err = bpf_loop(STROBE_MAX_MAPS, read_var_callback, &ctx, 0);
        if (err != STROBE_MAX_MAPS)
                return NULL;
+
+       payload_off = ctx.payload_off;
+       /* this should not really happen, here only to satisfy verifer */
+       if (payload_off > sizeof(data->payload))
+               payload_off = sizeof(data->payload);
 #else
 #ifdef NO_UNROLL
 #pragma clang loop unroll(disable)
@@ -555,7 +573,7 @@ static void *read_strobe_meta(struct task_struct *task,
 #pragma unroll
 #endif /* NO_UNROLL */
        for (int i = 0; i < STROBE_MAX_STRS; ++i) {
-               payload += read_str_var(cfg, i, tls_base, &value, data, payload);
+               payload_off = read_str_var(cfg, i, tls_base, &value, data, payload_off);
        }
 #ifdef NO_UNROLL
 #pragma clang loop unroll(disable)
@@ -563,7 +581,7 @@ static void *read_strobe_meta(struct task_struct *task,
 #pragma unroll
 #endif /* NO_UNROLL */
        for (int i = 0; i < STROBE_MAX_MAPS; ++i) {
-               payload = read_map_var(cfg, i, tls_base, &value, data, payload);
+               payload_off = read_map_var(cfg, i, tls_base, &value, data, payload_off);
        }
 #endif /* USE_BPF_LOOP */
 
@@ -571,7 +589,7 @@ static void *read_strobe_meta(struct task_struct *task,
         * return pointer right after end of payload, so it's possible to
         * calculate exact amount of useful data that needs to be sent
         */
-       return payload;
+       return &data->payload[payload_off];
 }
 
 SEC("raw_tracepoint/kfree_skb")