diff options
author | Daniel Borkmann <[email protected]> | 2021-12-07 12:51:56 +0000 |
---|---|---|
committer | Alexei Starovoitov <[email protected]> | 2021-12-14 19:33:06 -0800 |
commit | 7d3baf0afa3aa9102d6a521a8e4c41888bb79882 (patch) | |
tree | 34fa7b5f95965dc2cdc69198babd2274a2f21143 /net/lapb/lapb_timer.c | |
parent | f7abc4c8df8c7930d0b9c56d9abee9a1fca635e9 (diff) |
bpf: Fix kernel address leakage in atomic fetch
The change in commit 37086bfdc737 ("bpf: Propagate stack bounds to registers
in atomics w/ BPF_FETCH") around check_mem_access() handling is buggy since
this would allow for unprivileged users to leak kernel pointers. For example,
an atomic fetch/and with -1 on a stack destination which holds a spilled
pointer will migrate the spilled register type into a scalar, which can then
be exported out of the program (since scalar != pointer) by dumping it into
a map value.
The original implementation of XADD was preventing this situation by using
a double call to check_mem_access() one with BPF_READ and a subsequent one
with BPF_WRITE, in both cases passing -1 as a placeholder value instead of
register as per XADD semantics since it didn't contain a value fetch. The
BPF_READ also included a check in check_stack_read_fixed_off() which rejects
the program if the stack slot is of __is_pointer_value() if dst_regno < 0.
The latter is to distinguish whether we're dealing with a regular stack spill/
fill or some arithmetical operation which is disallowed on non-scalars, see
also 6e7e63cbb023 ("bpf: Forbid XADD on spilled pointers for unprivileged
users") for more context on check_mem_access() and its handling of placeholder
value -1.
One minimally intrusive option to fix the leak is for the BPF_FETCH case to
initially check the BPF_READ case via check_mem_access() with -1 as register,
followed by the actual load case with non-negative load_reg to propagate
stack bounds to registers.
Fixes: 37086bfdc737 ("bpf: Propagate stack bounds to registers in atomics w/ BPF_FETCH")
Reported-by: <[email protected]>
Acked-by: Brendan Jackman <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Diffstat (limited to 'net/lapb/lapb_timer.c')
0 files changed, 0 insertions, 0 deletions