Per-CPU array, hash, and cgroup storage map updates without BPF_F_CPU
or BPF_F_ALL_CPUS use a value buffer whose per-CPU slots are packed in
possible-CPU order. The buffer is sized as:

  round_up(value_size, 8) * num_possible_cpus()

The update paths iterate over possible CPUs, but use the logical CPU ID
to calculate the source offset:

  value + size * cpu

This only works when possible CPU IDs are contiguous starting at zero.

For example, with a possible CPU mask of 0,2-3, the buffer contains
three slots corresponding to CPUs 0, 2, and 3. CPU2 is therefore
expected to use slot 1 and CPU3 slot 2. Instead, the current code uses
slots 2 and 3 respectively, causing incorrect per-CPU values and an
out-of-bounds read from the update buffer for CPU3.

The corresponding lookup paths already use a dense offset while
iterating over possible CPUs. Do the same for the array, hash, and
cgroup storage update paths, advancing the source offset once for each
possible CPU. BPF_F_ALL_CPUS continues to use the same value for every
CPU.

Fixes: 8eb76cb03f0f ("bpf: Add BPF_F_CPU and BPF_F_ALL_CPUS flags support for 
percpu_array maps")
Reported-by: [email protected]
Signed-off-by: Hui Su <[email protected]>
---
 kernel/bpf/arraymap.c      | 5 +++--
 kernel/bpf/hashtab.c       | 4 +++-
 kernel/bpf/local_storage.c | 5 +++--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 248b4818178c..cc3f8c25a28b 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -405,7 +405,7 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, 
void *value,
        void __percpu *pptr;
        void *ptr, *val;
        u32 size;
-       int cpu;
+       int cpu, off = 0;
 
        if (unlikely((map_flags & BPF_F_LOCK) || (u32)map_flags > 
BPF_F_ALL_CPUS))
                /* unknown flags */
@@ -437,9 +437,10 @@ int bpf_percpu_array_update(struct bpf_map *map, void 
*key, void *value,
        }
        for_each_possible_cpu(cpu) {
                ptr = per_cpu_ptr(pptr, cpu);
-               val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
+               val = (map_flags & BPF_F_ALL_CPUS) ? value : value + off;
                copy_map_value(map, ptr, val);
                bpf_obj_cancel_fields(map, ptr);
+               off += size;
        }
 unlock:
        rcu_read_unlock();
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 9f394e1aa2e8..298b16ac4cc0 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1026,6 +1026,7 @@ static void pcpu_copy_value(struct bpf_htab *htab, void 
__percpu *pptr,
        } else {
                u32 size = round_up(htab->map.value_size, 8);
                void *val;
+               int off = 0;
                int cpu;
 
                if (map_flags & BPF_F_CPU) {
@@ -1038,9 +1039,10 @@ static void pcpu_copy_value(struct bpf_htab *htab, void 
__percpu *pptr,
 
                for_each_possible_cpu(cpu) {
                        ptr = per_cpu_ptr(pptr, cpu);
-                       val = (map_flags & BPF_F_ALL_CPUS) ? value : value + 
size * cpu;
+                       val = (map_flags & BPF_F_ALL_CPUS) ? value : value + 
off;
                        copy_map_value(&htab->map, ptr, val);
                        bpf_obj_cancel_fields(&htab->map, ptr);
+                       off += size;
                }
        }
 }
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 23267213a17f..83cd527a2542 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -220,7 +220,7 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, 
void *key,
        struct bpf_cgroup_storage *storage;
        void *val;
        u32 size;
-       int cpu;
+       int cpu, off = 0;
 
        if ((u32)map_flags & ~(BPF_ANY | BPF_EXIST | BPF_F_CPU | 
BPF_F_ALL_CPUS))
                return -EINVAL;
@@ -245,8 +245,9 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, 
void *key,
        }
        size = round_up(_map->value_size, 8);
        for_each_possible_cpu(cpu) {
-               val = (map_flags & BPF_F_ALL_CPUS) ? value : value + size * cpu;
+               val = (map_flags & BPF_F_ALL_CPUS) ? value : value + off;
                copy_map_value(_map, per_cpu_ptr(storage->percpu_buf, cpu), 
val);
+               off += size;
        }
 unlock:
        rcu_read_unlock();
-- 
2.54.0


Reply via email to