• Baokun Li's avatar
    quota: fix dqput() to follow the guarantees dquot_srcu should provide · dabc8b20
    Baokun Li authored
    The dquot_mark_dquot_dirty() using dquot references from the inode
    should be protected by dquot_srcu. quota_off code takes care to call
    synchronize_srcu(&dquot_srcu) to not drop dquot references while they
    are used by other users. But dquot_transfer() breaks this assumption.
    We call dquot_transfer() to drop the last reference of dquot and add
    it to free_dquots, but there may still be other users using the dquot
    at this time, as shown in the function graph below:
    
           cpu1              cpu2
    _________________|_________________
    wb_do_writeback         CHOWN(1)
     ...
      ext4_da_update_reserve_space
       dquot_claim_block
        ...
         dquot_mark_dquot_dirty // try to dirty old quota
          test_bit(DQ_ACTIVE_B, &dquot->dq_flags) // still ACTIVE
          if (test_bit(DQ_MOD_B, &dquot->dq_flags))
          // test no dirty, wait dq_list_lock
                        ...
                         dquot_transfer
                          __dquot_transfer
                          dqput_all(transfer_from) // rls old dquot
                           dqput // last dqput
                            dquot_release
                             clear_bit(DQ_ACTIVE_B, &dquot->dq_flags)
                            atomic_dec(&dquot->dq_count)
                            put_dquot_last(dquot)
                             list_add_tail(&dquot->dq_free, &free_dquots)
                             // add the dquot to free_dquots
          if (!test_and_set_bit(DQ_MOD_B, &dquot->dq_flags))
            add dqi_dirty_list // add released dquot to dirty_list
    
    This can cause various issues, such as dquot being destroyed by
    dqcache_shrink_scan() after being added to free_dquots, which can trigger
    a UAF in dquot_mark_dquot_dirty(); or after dquot is added to free_dquots
    and then to dirty_list, it is added to free_dquots again after
    dquot_writeback_dquots() is executed, which causes the free_dquots list to
    be corrupted and triggers a UAF when dqcache_shrink_scan() is called for
    freeing dquot twice.
    
    As Honza said, we need to fix dquot_transfer() to follow the guarantees
    dquot_srcu should provide. But calling synchronize_srcu() directly from
    dquot_transfer() is too expensive (and mostly unnecessary). So we add
    dquot whose last reference should be dropped to the new global dquot
    list releasing_dquots, and then queue work item which would call
    synchronize_srcu() and after that perform the final cleanup of all the
    dquots on releasing_dquots.
    
    Fixes: 4580b30e ("quota: Do not dirty bad dquots")
    Suggested-by: default avatarJan Kara <jack@suse.cz>
    Signed-off-by: default avatarBaokun Li <libaokun1@huawei.com>
    Signed-off-by: default avatarJan Kara <jack@suse.cz>
    Message-Id: <20230630110822.3881712-5-libaokun1@huawei.com>
    dabc8b20
dquot.c 81 KB