• Dave Marchevsky's avatar
    bpf: Fix release_on_unlock release logic for multiple refs · 1f82dffc
    Dave Marchevsky authored
    Consider a verifier state with three acquired references, all with
    release_on_unlock = true:
    
                idx  0 1 2
      state->refs = [2 4 6]
    
    (with 2, 4, and 6 being the ref ids).
    
    When bpf_spin_unlock is called, process_spin_lock will loop through all
    acquired_refs and, for each ref, if it's release_on_unlock, calls
    release_reference on it. That function in turn calls
    release_reference_state, which removes the reference from state->refs by
    swapping the reference state with the last reference state in
    refs array and decrements acquired_refs count.
    
    process_spin_lock's loop logic, which is essentially:
    
      for (i = 0; i < state->acquired_refs; i++) {
        if (!state->refs[i].release_on_unlock)
          continue;
        release_reference(state->refs[i].id);
      }
    
    will fail to release release_on_unlock references which are swapped from
    the end. Running this logic on our example demonstrates:
    
      state->refs = [2 4 6] (start of idx=0 iter)
        release state->refs[0] by swapping w/ state->refs[2]
    
      state->refs = [6 4]   (start of idx=1)
        release state->refs[1], no need to swap as it's the last idx
    
      state->refs = [6]     (start of idx=2, loop terminates)
    
    ref_id 6 should have been removed but was skipped.
    
    Fix this by looping from back-to-front, which results in refs that are
    candidates for removal being swapped with refs which have already been
    examined and kept.
    
    If we modify our initial example such that ref 6 is replaced with ref 7,
    which is _not_ release_on_unlock, and loop from the back, we'd see:
    
      state->refs = [2 4 7] (start of idx=2)
    
      state->refs = [2 4 7] (start of idx=1)
    
      state->refs = [2 7]   (start of idx=0, refs 7 and 4 swapped)
    
      state->refs = [7]     (after idx=0, 7 and 2 swapped, loop terminates)
    Signed-off-by: default avatarDave Marchevsky <davemarchevsky@fb.com>
    Acked-by: default avatarYonghong Song <yhs@fb.com>
    cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
    Fixes: 534e86bc ("bpf: Add 'release on unlock' logic for bpf_list_push_{front,back}")
    Link: https://lore.kernel.org/r/20221201183406.1203621-1-davemarchevsky@fb.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    1f82dffc
verifier.c 487 KB