Skip to content

Commit dabc8b2

Browse files
LiBaokun96jankara
authored andcommitted
quota: fix dqput() to follow the guarantees dquot_srcu should provide
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: 4580b30 ("quota: Do not dirty bad dquots") Suggested-by: Jan Kara <jack@suse.cz> Signed-off-by: Baokun Li <libaokun1@huawei.com> Signed-off-by: Jan Kara <jack@suse.cz> Message-Id: <20230630110822.3881712-5-libaokun1@huawei.com>
1 parent 33bcfaf commit dabc8b2

File tree

1 file changed

+78
-18
lines changed

1 file changed

+78
-18
lines changed

fs/quota/dquot.c

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,22 @@ static void put_quota_format(struct quota_format_type *fmt)
225225

226226
/*
227227
* Dquot List Management:
228-
* The quota code uses four lists for dquot management: the inuse_list,
229-
* free_dquots, dqi_dirty_list, and dquot_hash[] array. A single dquot
230-
* structure may be on some of those lists, depending on its current state.
228+
* The quota code uses five lists for dquot management: the inuse_list,
229+
* releasing_dquots, free_dquots, dqi_dirty_list, and dquot_hash[] array.
230+
* A single dquot structure may be on some of those lists, depending on
231+
* its current state.
231232
*
232233
* All dquots are placed to the end of inuse_list when first created, and this
233234
* list is used for invalidate operation, which must look at every dquot.
234235
*
236+
* When the last reference of a dquot will be dropped, the dquot will be
237+
* added to releasing_dquots. We'd then queue work item which would call
238+
* synchronize_srcu() and after that perform the final cleanup of all the
239+
* dquots on the list. Both releasing_dquots and free_dquots use the
240+
* dq_free list_head in the dquot struct. When a dquot is removed from
241+
* releasing_dquots, a reference count is always subtracted, and if
242+
* dq_count == 0 at that point, the dquot will be added to the free_dquots.
243+
*
235244
* Unused dquots (dq_count == 0) are added to the free_dquots list when freed,
236245
* and this list is searched whenever we need an available dquot. Dquots are
237246
* removed from the list as soon as they are used again, and
@@ -250,6 +259,7 @@ static void put_quota_format(struct quota_format_type *fmt)
250259

251260
static LIST_HEAD(inuse_list);
252261
static LIST_HEAD(free_dquots);
262+
static LIST_HEAD(releasing_dquots);
253263
static unsigned int dq_hash_bits, dq_hash_mask;
254264
static struct hlist_head *dquot_hash;
255265

@@ -260,6 +270,9 @@ static qsize_t inode_get_rsv_space(struct inode *inode);
260270
static qsize_t __inode_get_rsv_space(struct inode *inode);
261271
static int __dquot_initialize(struct inode *inode, int type);
262272

273+
static void quota_release_workfn(struct work_struct *work);
274+
static DECLARE_DELAYED_WORK(quota_release_work, quota_release_workfn);
275+
263276
static inline unsigned int
264277
hashfn(const struct super_block *sb, struct kqid qid)
265278
{
@@ -305,12 +318,18 @@ static inline void put_dquot_last(struct dquot *dquot)
305318
dqstats_inc(DQST_FREE_DQUOTS);
306319
}
307320

321+
static inline void put_releasing_dquots(struct dquot *dquot)
322+
{
323+
list_add_tail(&dquot->dq_free, &releasing_dquots);
324+
}
325+
308326
static inline void remove_free_dquot(struct dquot *dquot)
309327
{
310328
if (list_empty(&dquot->dq_free))
311329
return;
312330
list_del_init(&dquot->dq_free);
313-
dqstats_dec(DQST_FREE_DQUOTS);
331+
if (!atomic_read(&dquot->dq_count))
332+
dqstats_dec(DQST_FREE_DQUOTS);
314333
}
315334

316335
static inline void put_inuse(struct dquot *dquot)
@@ -552,6 +571,8 @@ static void invalidate_dquots(struct super_block *sb, int type)
552571
struct dquot *dquot, *tmp;
553572

554573
restart:
574+
flush_delayed_work(&quota_release_work);
575+
555576
spin_lock(&dq_list_lock);
556577
list_for_each_entry_safe(dquot, tmp, &inuse_list, dq_inuse) {
557578
if (dquot->dq_sb != sb)
@@ -560,6 +581,12 @@ static void invalidate_dquots(struct super_block *sb, int type)
560581
continue;
561582
/* Wait for dquot users */
562583
if (atomic_read(&dquot->dq_count)) {
584+
/* dquot in releasing_dquots, flush and retry */
585+
if (!list_empty(&dquot->dq_free)) {
586+
spin_unlock(&dq_list_lock);
587+
goto restart;
588+
}
589+
563590
atomic_inc(&dquot->dq_count);
564591
spin_unlock(&dq_list_lock);
565592
/*
@@ -770,6 +797,49 @@ static struct shrinker dqcache_shrinker = {
770797
.seeks = DEFAULT_SEEKS,
771798
};
772799

800+
/*
801+
* Safely release dquot and put reference to dquot.
802+
*/
803+
static void quota_release_workfn(struct work_struct *work)
804+
{
805+
struct dquot *dquot;
806+
struct list_head rls_head;
807+
808+
spin_lock(&dq_list_lock);
809+
/* Exchange the list head to avoid livelock. */
810+
list_replace_init(&releasing_dquots, &rls_head);
811+
spin_unlock(&dq_list_lock);
812+
813+
restart:
814+
synchronize_srcu(&dquot_srcu);
815+
spin_lock(&dq_list_lock);
816+
while (!list_empty(&rls_head)) {
817+
dquot = list_first_entry(&rls_head, struct dquot, dq_free);
818+
/* Dquot got used again? */
819+
if (atomic_read(&dquot->dq_count) > 1) {
820+
remove_free_dquot(dquot);
821+
atomic_dec(&dquot->dq_count);
822+
continue;
823+
}
824+
if (dquot_dirty(dquot)) {
825+
spin_unlock(&dq_list_lock);
826+
/* Commit dquot before releasing */
827+
dquot_write_dquot(dquot);
828+
goto restart;
829+
}
830+
if (dquot_active(dquot)) {
831+
spin_unlock(&dq_list_lock);
832+
dquot->dq_sb->dq_op->release_dquot(dquot);
833+
goto restart;
834+
}
835+
/* Dquot is inactive and clean, now move it to free list */
836+
remove_free_dquot(dquot);
837+
atomic_dec(&dquot->dq_count);
838+
put_dquot_last(dquot);
839+
}
840+
spin_unlock(&dq_list_lock);
841+
}
842+
773843
/*
774844
* Put reference to dquot
775845
*/
@@ -786,7 +856,7 @@ void dqput(struct dquot *dquot)
786856
}
787857
#endif
788858
dqstats_inc(DQST_DROPS);
789-
we_slept:
859+
790860
spin_lock(&dq_list_lock);
791861
if (atomic_read(&dquot->dq_count) > 1) {
792862
/* We have more than one user... nothing to do */
@@ -798,25 +868,15 @@ void dqput(struct dquot *dquot)
798868
spin_unlock(&dq_list_lock);
799869
return;
800870
}
871+
801872
/* Need to release dquot? */
802-
if (dquot_dirty(dquot)) {
803-
spin_unlock(&dq_list_lock);
804-
/* Commit dquot before releasing */
805-
dquot_write_dquot(dquot);
806-
goto we_slept;
807-
}
808-
if (dquot_active(dquot)) {
809-
spin_unlock(&dq_list_lock);
810-
dquot->dq_sb->dq_op->release_dquot(dquot);
811-
goto we_slept;
812-
}
813-
atomic_dec(&dquot->dq_count);
814873
#ifdef CONFIG_QUOTA_DEBUG
815874
/* sanity check */
816875
BUG_ON(!list_empty(&dquot->dq_free));
817876
#endif
818-
put_dquot_last(dquot);
877+
put_releasing_dquots(dquot);
819878
spin_unlock(&dq_list_lock);
879+
queue_delayed_work(system_unbound_wq, &quota_release_work, 1);
820880
}
821881
EXPORT_SYMBOL(dqput);
822882

0 commit comments

Comments
 (0)