• Boris Burkov's avatar
    btrfs: fix start transaction qgroup rsv double free · a6496849
    Boris Burkov authored
    btrfs_start_transaction reserves metadata space of the PERTRANS type
    before it identifies a transaction to start/join. This allows flushing
    when reserving that space without a deadlock. However, it results in a
    race which temporarily breaks qgroup rsv accounting.
    
    T1                                              T2
    start_transaction
    do_stuff
                                                start_transaction
                                                    qgroup_reserve_meta_pertrans
    commit_transaction
        qgroup_free_meta_all_pertrans
                                                hit an error starting txn
                                                goto reserve_fail
                                                qgroup_free_meta_pertrans (already freed!)
    
    The basic issue is that there is nothing preventing another commit from
    committing before start_transaction finishes (in fact sometimes we
    intentionally wait for it) so any error path that frees the reserve is
    at risk of this race.
    
    While this exact space was getting freed anyway, and it's not a huge
    deal to double free it (just a warning, the free code catches this), it
    can result in incorrectly freeing some other pertrans reservation in
    this same reservation, which could then lead to spuriously granting
    reservations we might not have the space for. Therefore, I do believe it
    is worth fixing.
    
    To fix it, use the existing prealloc->pertrans conversion mechanism.
    When we first reserve the space, we reserve prealloc space and only when
    we are sure we have a transaction do we convert it to pertrans. This way
    any racing commits do not blow away our reservation, but we still get a
    pertrans reservation that is freed when _this_ transaction gets committed.
    
    This issue can be reproduced by running generic/269 with either qgroups
    or squotas enabled via mkfs on the scratch device.
    Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
    CC: stable@vger.kernel.org # 5.10+
    Signed-off-by: default avatarBoris Burkov <boris@bur.io>
    Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
    a6496849
transaction.c 78.1 KB