From: Chengguang Xu cgxu519@gmail.com

mainline inclusion from mainline-v5.3-rc1 commit f44840ad1f822d9ecee6a3f91f2d17825a361307 category: bugfix bugzilla: 188812,https://gitee.com/openeuler/kernel/issues/I7E0YR CVE: NA

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...

--------------------------------

Actually there are four lists for dquot management, so add the description of dqui_dirty_list to comment.

Signed-off-by: Chengguang Xu cgxu519@gmail.com Signed-off-by: Jan Kara jack@suse.cz Signed-off-by: Baokun Li libaokun1@huawei.com Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- fs/quota/dquot.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 3b02d68d9522..b0e676383975 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -223,9 +223,9 @@ static void put_quota_format(struct quota_format_type *fmt)

/* * Dquot List Management: - * The quota code uses three lists for dquot management: the inuse_list, - * free_dquots, and dquot_hash[] array. A single dquot structure may be - * on all three lists, depending on its current state. + * The quota code uses four lists for dquot management: the inuse_list, + * free_dquots, dqi_dirty_list, and dquot_hash[] array. A single dquot + * structure may be on some of those lists, depending on its current state. * * All dquots are placed to the end of inuse_list when first created, and this * list is used for invalidate operation, which must look at every dquot. @@ -236,6 +236,11 @@ static void put_quota_format(struct quota_format_type *fmt) * dqstats.free_dquots gives the number of dquots on the list. When * dquot is invalidated it's completely released from memory. * + * Dirty dquots are added to the dqi_dirty_list of quota_info when mark + * dirtied, and this list is searched when writing dirty dquots back to + * quota file. Note that some filesystems do dirty dquot tracking on their + * own (e.g. in a journal) and thus don't use dqi_dirty_list. + * * Dquots with a specific identity (device, type and id) are placed on * one of the dquot_hash[] hash chains. The provides an efficient search * mechanism to locate a specific dquot.

From: Baokun Li libaokun1@huawei.com

maillist inclusion category: bugfix bugzilla: 188812,https://gitee.com/openeuler/kernel/issues/I7E0YR CVE: NA

Reference: https://www.spinics.net/lists/kernel/msg4844759.html

----------------------------------------

Now we have a helper function dquot_dirty() to determine if dquot has DQ_MOD_B bit. dquot_active() can easily be misunderstood as a helper function to determine if dquot has DQ_ACTIVE_B bit. So we avoid this by renaming it to inode_quota_active() and later on we will add the helper function dquot_active() to determine if dquot has DQ_ACTIVE_B bit.

Signed-off-by: Baokun Li libaokun1@huawei.com Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- fs/quota/dquot.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 62f5adb23b33..b6eb54095e97 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -1404,7 +1404,7 @@ static int info_bdq_free(struct dquot *dquot, qsize_t space) return QUOTA_NL_NOWARN; }

-static int dquot_active(const struct inode *inode) +static int inode_quota_active(const struct inode *inode) { struct super_block *sb = inode->i_sb;

@@ -1427,7 +1427,7 @@ static int __dquot_initialize(struct inode *inode, int type) qsize_t rsv; int ret = 0;

- if (!dquot_active(inode)) + if (!inode_quota_active(inode)) return 0;

dquots = i_dquot(inode); @@ -1535,7 +1535,7 @@ bool dquot_initialize_needed(struct inode *inode) struct dquot **dquots; int i;

- if (!dquot_active(inode)) + if (!inode_quota_active(inode)) return false;

dquots = i_dquot(inode); @@ -1646,7 +1646,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number, int flags) int reserve = flags & DQUOT_SPACE_RESERVE; struct dquot **dquots;

- if (!dquot_active(inode)) { + if (!inode_quota_active(inode)) { if (reserve) { spin_lock(&inode->i_lock); *inode_reserved_space(inode) += number; @@ -1718,7 +1718,7 @@ int dquot_alloc_inode(struct inode *inode) struct dquot_warn warn[MAXQUOTAS]; struct dquot * const *dquots;

- if (!dquot_active(inode)) + if (!inode_quota_active(inode)) return 0; for (cnt = 0; cnt < MAXQUOTAS; cnt++) warn[cnt].w_type = QUOTA_NL_NOWARN; @@ -1761,7 +1761,7 @@ int dquot_claim_space_nodirty(struct inode *inode, qsize_t number) struct dquot **dquots; int cnt, index;

- if (!dquot_active(inode)) { + if (!inode_quota_active(inode)) { spin_lock(&inode->i_lock); *inode_reserved_space(inode) -= number; __inode_add_bytes(inode, number); @@ -1803,7 +1803,7 @@ void dquot_reclaim_space_nodirty(struct inode *inode, qsize_t number) struct dquot **dquots; int cnt, index;

- if (!dquot_active(inode)) { + if (!inode_quota_active(inode)) { spin_lock(&inode->i_lock); *inode_reserved_space(inode) += number; __inode_sub_bytes(inode, number); @@ -1847,7 +1847,7 @@ void __dquot_free_space(struct inode *inode, qsize_t number, int flags) struct dquot **dquots; int reserve = flags & DQUOT_SPACE_RESERVE, index;

- if (!dquot_active(inode)) { + if (!inode_quota_active(inode)) { if (reserve) { spin_lock(&inode->i_lock); *inode_reserved_space(inode) -= number; @@ -1902,7 +1902,7 @@ void dquot_free_inode(struct inode *inode) struct dquot * const *dquots; int index;

- if (!dquot_active(inode)) + if (!inode_quota_active(inode)) return;

dquots = i_dquot(inode); @@ -2073,7 +2073,7 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr) struct super_block *sb = inode->i_sb; int ret;

- if (!dquot_active(inode)) + if (!inode_quota_active(inode)) return 0;

if (iattr->ia_valid & ATTR_UID && !uid_eq(iattr->ia_uid, inode->i_uid)){

From: Baokun Li libaokun1@huawei.com

maillist inclusion category: bugfix bugzilla: 188812,https://gitee.com/openeuler/kernel/issues/I7E0YR CVE: NA

Reference: https://www.spinics.net/lists/kernel/msg4844759.html

----------------------------------------

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: 4580b30ea887 ("quota: Do not dirty bad dquots") Suggested-by: Jan Kara jack@suse.cz Signed-off-by: Baokun Li libaokun1@huawei.com Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- fs/quota/dquot.c | 96 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 78 insertions(+), 18 deletions(-)

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index c1c056faea2c..4f077580baa0 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -223,13 +223,22 @@ static void put_quota_format(struct quota_format_type *fmt)

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

static LIST_HEAD(inuse_list); static LIST_HEAD(free_dquots); +static LIST_HEAD(releasing_dquots); static unsigned int dq_hash_bits, dq_hash_mask; static struct hlist_head *dquot_hash;

@@ -258,6 +268,9 @@ static qsize_t inode_get_rsv_space(struct inode *inode); static qsize_t __inode_get_rsv_space(struct inode *inode); static int __dquot_initialize(struct inode *inode, int type);

+static void quota_release_workfn(struct work_struct *work); +static DECLARE_DELAYED_WORK(quota_release_work, quota_release_workfn); + static inline unsigned int hashfn(const struct super_block *sb, struct kqid qid) { @@ -305,12 +318,18 @@ static inline void put_dquot_last(struct dquot *dquot) dqstats_inc(DQST_FREE_DQUOTS); }

+static inline void put_releasing_dquots(struct dquot *dquot) +{ + list_add_tail(&dquot->dq_free, &releasing_dquots); +} + static inline void remove_free_dquot(struct dquot *dquot) { if (list_empty(&dquot->dq_free)) return; list_del_init(&dquot->dq_free); - dqstats_dec(DQST_FREE_DQUOTS); + if (!atomic_read(&dquot->dq_count)) + dqstats_dec(DQST_FREE_DQUOTS); }

static inline void put_inuse(struct dquot *dquot) @@ -542,6 +561,8 @@ static void invalidate_dquots(struct super_block *sb, int type) struct dquot *dquot, *tmp;

restart: + flush_delayed_work(&quota_release_work); + spin_lock(&dq_list_lock); list_for_each_entry_safe(dquot, tmp, &inuse_list, dq_inuse) { if (dquot->dq_sb != sb) @@ -550,6 +571,12 @@ static void invalidate_dquots(struct super_block *sb, int type) continue; /* Wait for dquot users */ if (atomic_read(&dquot->dq_count)) { + /* dquot in releasing_dquots, flush and retry */ + if (!list_empty(&dquot->dq_free)) { + spin_unlock(&dq_list_lock); + goto restart; + } + dqgrab(dquot); spin_unlock(&dq_list_lock); /* @@ -760,6 +787,49 @@ static struct shrinker dqcache_shrinker = { .seeks = DEFAULT_SEEKS, };

+/* + * Safely release dquot and put reference to dquot. + */ +static void quota_release_workfn(struct work_struct *work) +{ + struct dquot *dquot; + struct list_head rls_head; + + spin_lock(&dq_list_lock); + /* Exchange the list head to avoid livelock. */ + list_replace_init(&releasing_dquots, &rls_head); + spin_unlock(&dq_list_lock); + +restart: + synchronize_srcu(&dquot_srcu); + spin_lock(&dq_list_lock); + while (!list_empty(&rls_head)) { + dquot = list_first_entry(&rls_head, struct dquot, dq_free); + /* Dquot got used again? */ + if (atomic_read(&dquot->dq_count) > 1) { + remove_free_dquot(dquot); + atomic_dec(&dquot->dq_count); + continue; + } + if (dquot_dirty(dquot)) { + spin_unlock(&dq_list_lock); + /* Commit dquot before releasing */ + dquot_write_dquot(dquot); + goto restart; + } + if (dquot_active(dquot)) { + spin_unlock(&dq_list_lock); + dquot->dq_sb->dq_op->release_dquot(dquot); + goto restart; + } + /* Dquot is inactive and clean, now move it to free list */ + remove_free_dquot(dquot); + atomic_dec(&dquot->dq_count); + put_dquot_last(dquot); + } + spin_unlock(&dq_list_lock); +} + /* * Put reference to dquot */ @@ -776,7 +846,7 @@ void dqput(struct dquot *dquot) } #endif dqstats_inc(DQST_DROPS); -we_slept: + spin_lock(&dq_list_lock); if (atomic_read(&dquot->dq_count) > 1) { /* We have more than one user... nothing to do */ @@ -788,25 +858,15 @@ void dqput(struct dquot *dquot) spin_unlock(&dq_list_lock); return; } + /* Need to release dquot? */ - if (dquot_dirty(dquot)) { - spin_unlock(&dq_list_lock); - /* Commit dquot before releasing */ - dquot_write_dquot(dquot); - goto we_slept; - } - if (dquot_active(dquot)) { - spin_unlock(&dq_list_lock); - dquot->dq_sb->dq_op->release_dquot(dquot); - goto we_slept; - } - atomic_dec(&dquot->dq_count); #ifdef CONFIG_QUOTA_DEBUG /* sanity check */ BUG_ON(!list_empty(&dquot->dq_free)); #endif - put_dquot_last(dquot); + put_releasing_dquots(dquot); spin_unlock(&dq_list_lock); + queue_delayed_work(system_unbound_wq, &quota_release_work, 1); } EXPORT_SYMBOL(dqput);