• Andrei Matei's avatar
    bpf: Fix accesses to uninit stack slots · 6b4a64ba
    Andrei Matei authored
    Privileged programs are supposed to be able to read uninitialized stack
    memory (ever since 6715df8d) but, before this patch, these accesses
    were permitted inconsistently. In particular, accesses were permitted
    above state->allocated_stack, but not below it. In other words, if the
    stack was already "large enough", the access was permitted, but
    otherwise the access was rejected instead of being allowed to "grow the
    stack". This undesired rejection was happening in two places:
    - in check_stack_slot_within_bounds()
    - in check_stack_range_initialized()
    This patch arranges for these accesses to be permitted. A bunch of tests
    that were relying on the old rejection had to change; all of them were
    changed to add also run unprivileged, in which case the old behavior
    persists. One tests couldn't be updated - global_func16 - because it
    can't run unprivileged for other reasons.
    
    This patch also fixes the tracking of the stack size for variable-offset
    reads. This second fix is bundled in the same commit as the first one
    because they're inter-related. Before this patch, writes to the stack
    using registers containing a variable offset (as opposed to registers
    with fixed, known values) were not properly contributing to the
    function's needed stack size. As a result, it was possible for a program
    to verify, but then to attempt to read out-of-bounds data at runtime
    because a too small stack had been allocated for it.
    
    Each function tracks the size of the stack it needs in
    bpf_subprog_info.stack_depth, which is maintained by
    update_stack_depth(). For regular memory accesses, check_mem_access()
    was calling update_state_depth() but it was passing in only the fixed
    part of the offset register, ignoring the variable offset. This was
    incorrect; the minimum possible value of that register should be used
    instead.
    
    This tracking is now fixed by centralizing the tracking of stack size in
    grow_stack_state(), and by lifting the calls to grow_stack_state() to
    check_stack_access_within_bounds() as suggested by Andrii. The code is
    now simpler and more convincingly tracks the correct maximum stack size.
    check_stack_range_initialized() can now rely on enough stack having been
    allocated for the access; this helps with the fix for the first issue.
    
    A few tests were changed to also check the stack depth computation. The
    one that fails without this patch is verifier_var_off:stack_write_priv_vs_unpriv.
    
    Fixes: 01f810ac ("bpf: Allow variable-offset stack access")
    Reported-by: default avatarHao Sun <sunhao.th@gmail.com>
    Signed-off-by: default avatarAndrei Matei <andreimatei1@gmail.com>
    Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
    Acked-by: default avatarAndrii Nakryiko <andrii@kernel.org>
    Link: https://lore.kernel.org/bpf/20231208032519.260451-3-andreimatei1@gmail.com
    
    Closes: https://lore.kernel.org/bpf/CABWLsev9g8UP_c3a=1qbuZUi20tGoUXoU07FPf-5FLvhOKOY+Q@mail.gmail.com/
    6b4a64ba
atomic_cmpxchg.c 6.57 KB