linux/kernel/bpf
John Fastabend f263a81451 bpf: Track subprog poke descriptors correctly and fix use-after-free
Subprograms are calling map_poke_track(), but on program release there is no
hook to call map_poke_untrack(). However, on program release, the aux memory
(and poke descriptor table) is freed even though we still have a reference to
it in the element list of the map aux data. When we run map_poke_run(), we then
end up accessing free'd memory, triggering KASAN in prog_array_map_poke_run():

  [...]
  [  402.824689] BUG: KASAN: use-after-free in prog_array_map_poke_run+0xc2/0x34e
  [  402.824698] Read of size 4 at addr ffff8881905a7940 by task hubble-fgs/4337
  [  402.824705] CPU: 1 PID: 4337 Comm: hubble-fgs Tainted: G          I       5.12.0+ #399
  [  402.824715] Call Trace:
  [  402.824719]  dump_stack+0x93/0xc2
  [  402.824727]  print_address_description.constprop.0+0x1a/0x140
  [  402.824736]  ? prog_array_map_poke_run+0xc2/0x34e
  [  402.824740]  ? prog_array_map_poke_run+0xc2/0x34e
  [  402.824744]  kasan_report.cold+0x7c/0xd8
  [  402.824752]  ? prog_array_map_poke_run+0xc2/0x34e
  [  402.824757]  prog_array_map_poke_run+0xc2/0x34e
  [  402.824765]  bpf_fd_array_map_update_elem+0x124/0x1a0
  [...]

The elements concerned are walked as follows:

    for (i = 0; i < elem->aux->size_poke_tab; i++) {
           poke = &elem->aux->poke_tab[i];
    [...]

The access to size_poke_tab is a 4 byte read, verified by checking offsets
in the KASAN dump:

  [  402.825004] The buggy address belongs to the object at ffff8881905a7800
                 which belongs to the cache kmalloc-1k of size 1024
  [  402.825008] The buggy address is located 320 bytes inside of
                 1024-byte region [ffff8881905a7800, ffff8881905a7c00)

The pahole output of bpf_prog_aux:

  struct bpf_prog_aux {
    [...]
    /* --- cacheline 5 boundary (320 bytes) --- */
    u32                        size_poke_tab;        /*   320     4 */
    [...]

In general, subprograms do not necessarily manage their own data structures.
For example, BTF func_info and linfo are just pointers to the main program
structure. This allows reference counting and cleanup to be done on the latter
which simplifies their management a bit. The aux->poke_tab struct, however,
did not follow this logic. The initial proposed fix for this use-after-free
bug further embedded poke data tracking into the subprogram with proper
reference counting. However, Daniel and Alexei questioned why we were treating
these objects special; I agree, its unnecessary. The fix here removes the per
subprogram poke table allocation and map tracking and instead simply points
the aux->poke_tab pointer at the main programs poke table. This way, map
tracking is simplified to the main program and we do not need to manage them
per subprogram.

This also means, bpf_prog_free_deferred(), which unwinds the program reference
counting and kfrees objects, needs to ensure that we don't try to double free
the poke_tab when free'ing the subprog structures. This is easily solved by
NULL'ing the poke_tab pointer. The second detail is to ensure that per
subprogram JIT logic only does fixups on poke_tab[] entries it owns. To do
this, we add a pointer in the poke structure to point at the subprogram value
so JITs can easily check while walking the poke_tab structure if the current
entry belongs to the current program. The aux pointer is stable and therefore
suitable for such comparison. On the jit_subprogs() error path, we omit
cleaning up the poke->aux field because these are only ever referenced from
the JIT side, but on error we will never make it to the JIT, so its fine to
leave them dangling. Removing these pointers would complicate the error path
for no reason. However, we do need to untrack all poke descriptors from the
main program as otherwise they could race with the freeing of JIT memory from
the subprograms. Lastly, a748c6975d ("bpf: propagate poke descriptors to
subprograms") had an off-by-one on the subprogram instruction index range
check as it was testing 'insn_idx >= subprog_start && insn_idx <= subprog_end'.
However, subprog_end is the next subprogram's start instruction.

Fixes: a748c6975d ("bpf: propagate poke descriptors to subprograms")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Co-developed-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20210707223848.14580-2-john.fastabend@gmail.com
2021-07-09 12:08:27 +02:00
..
preload libbpf: Move BPF_SEQ_PRINTF and BPF_SNPRINTF to bpf_helpers.h 2021-05-26 10:45:41 -07:00
arraymap.c bpf: Add batched ops support for percpu array 2021-04-28 01:17:45 +02:00
bpf_inode_storage.c bpf: Fix spelling mistakes 2021-05-24 21:13:05 -07:00
bpf_iter.c bpf: Prepare bpf syscall to be used from kernel and user space. 2021-05-19 00:33:40 +02:00
bpf_local_storage.c bpf: Prevent deadlock from recursive bpf_task_storage_[get|delete] 2021-02-26 11:51:48 -08:00
bpf_lru_list.c bpf_lru_list: Read double-checked variable once without lock 2021-02-10 15:54:26 -08:00
bpf_lru_list.h
bpf_lsm.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next 2021-06-17 11:54:56 -07:00
bpf_struct_ops_types.h
bpf_struct_ops.c bpf: Fix fexit trampoline. 2021-03-18 00:22:51 +01:00
bpf_task_storage.c bpf: Make symbol 'bpf_task_storage_busy' static 2021-03-16 12:24:20 -07:00
btf.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next 2021-06-17 11:54:56 -07:00
cgroup.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next 2021-02-16 13:14:06 -08:00
core.c bpf: Track subprog poke descriptors correctly and fix use-after-free 2021-07-09 12:08:27 +02:00
cpumap.c xdp: Add proper __rcu annotations to redirect map entries 2021-06-24 19:41:15 +02:00
devmap.c bpf, devmap: Convert remaining READ_ONCE() to rcu_dereference_check() 2021-07-01 09:28:38 +02:00
disasm.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net 2021-04-09 20:48:35 -07:00
disasm.h
dispatcher.c
hashtab.c bpf: Allow RCU-protected lookups to happen from bh context 2021-06-24 19:41:15 +02:00
helpers.c bpf: Allow RCU-protected lookups to happen from bh context 2021-06-24 19:41:15 +02:00
inode.c bpf: Fix regression on BPF_OBJ_GET with non-O_RDWR flags 2021-06-22 14:57:43 +02:00
Kconfig bpf: Fix BPF_JIT kconfig symbol dependency 2021-05-20 23:48:37 +02:00
local_storage.c bpf: Fix NULL pointer dereference in bpf_get_local_storage() helper 2021-03-25 18:31:36 -07:00
lpm_trie.c bpf: Allow RCU-protected lookups to happen from bh context 2021-06-24 19:41:15 +02:00
Makefile bpf: Enable task local storage for tracing programs 2021-02-26 11:51:47 -08:00
map_in_map.c bpf: Relax max_entries check for most of the inner map types 2020-08-28 15:41:30 +02:00
map_in_map.h bpf: Add map_meta_equal map ops 2020-08-28 15:41:30 +02:00
map_iter.c bpf: Implement link_query callbacks in map element iterators 2020-08-21 14:01:39 -07:00
net_namespace.c
offload.c
percpu_freelist.c bpf: Use raw_spin_trylock() for pcpu_freelist_push/pop in NMI 2020-10-06 00:04:11 +02:00
percpu_freelist.h bpf: Use raw_spin_trylock() for pcpu_freelist_push/pop in NMI 2020-10-06 00:04:11 +02:00
prog_iter.c
queue_stack_maps.c bpf: Eliminate rlimit-based memory accounting for queue_stack_maps maps 2020-12-02 18:32:46 -08:00
reuseport_array.c bpf: Fix spelling mistakes 2021-05-24 21:13:05 -07:00
ringbuf.c bpf: Fix false positive kmemleak report in bpf_ringbuf_area_alloc() 2021-06-28 15:57:46 +02:00
stackmap.c bpf: Refcount task stack in bpf_get_task_stack 2021-04-01 13:58:07 -07:00
syscall.c Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next 2021-06-17 11:54:56 -07:00
sysfs_btf.c bpf: Load and verify kernel module BTFs 2020-11-10 15:25:53 -08:00
task_iter.c bpf: Introduce task_vma bpf_iter 2021-02-12 12:56:53 -08:00
tnum.c bpf, tnums: Provably sound, faster, and more precise algorithm for tnum_mul 2021-06-01 13:34:15 +02:00
trampoline.c bpf: Fix spelling mistakes 2021-05-24 21:13:05 -07:00
verifier.c bpf: Track subprog poke descriptors correctly and fix use-after-free 2021-07-09 12:08:27 +02:00