quota: Fix potential NULL pointer dereference

[ Upstream commit d0aa72604fbd80c8aabb46eda00535ed35570f1f ]

Below race may cause NULL pointer dereference

P1					P2
dquot_free_inode			quota_off
					  drop_dquot_ref
					   remove_dquot_ref
					   dquots = i_dquot(inode)
  dquots = i_dquot(inode)
  srcu_read_lock
  dquots[cnt]) != NULL (1)
					     dquots[type] = NULL (2)
  spin_lock(&dquots[cnt]->dq_dqb_lock) (3)
   ....

If dquot_free_inode(or other routines) checks inode's quota pointers (1)
before quota_off sets it to NULL(2) and use it (3) after that, NULL pointer
dereference will be triggered.

So let's fix it by using a temporary pointer to avoid this issue.

Signed-off-by: Wang Jianjian <wangjianjian3@huawei.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Message-Id: <20240202081852.2514092-1-wangjianjian3@huawei.com>
Stable-dep-of: 179b8c97ebf6 ("quota: Fix rcu annotations of inode dquot pointers")
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 77578332e35ae2ee05f68326d9685345b4644298..3f19ef2cc186dbf0aa9124f8bc9b3da9a648fb5c 100644 (file)
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -401,15 +401,17 @@ int dquot_mark_dquot_dirty(struct dquot *dquot)
 EXPORT_SYMBOL(dquot_mark_dquot_dirty);
 
 /* Dirtify all the dquots - this can block when journalling */
-static inline int mark_all_dquot_dirty(struct dquot * const *dquot)
+static inline int mark_all_dquot_dirty(struct dquot * const *dquots)
 {
        int ret, err, cnt;
+       struct dquot *dquot;
 
        ret = err = 0;
        for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-               if (dquot[cnt])
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (dquot)
                        /* Even in case of error we have to continue */
-                       ret = mark_dquot_dirty(dquot[cnt]);
+                       ret = mark_dquot_dirty(dquot);
                if (!err)
                        err = ret;
        }
@@ -1686,6 +1688,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number, int flags)
        struct dquot_warn warn[MAXQUOTAS];
        int reserve = flags & DQUOT_SPACE_RESERVE;
        struct dquot **dquots;
+       struct dquot *dquot;
 
        if (!inode_quota_active(inode)) {
                if (reserve) {
@@ -1705,27 +1708,26 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number, int flags)
        index = srcu_read_lock(&dquot_srcu);
        spin_lock(&inode->i_lock);
        for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-               if (!dquots[cnt])
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (!dquot)
                        continue;
                if (reserve) {
-                       ret = dquot_add_space(dquots[cnt], 0, number, flags,
-                                             &warn[cnt]);
+                       ret = dquot_add_space(dquot, 0, number, flags, &warn[cnt]);
                } else {
-                       ret = dquot_add_space(dquots[cnt], number, 0, flags,
-                                             &warn[cnt]);
+                       ret = dquot_add_space(dquot, number, 0, flags, &warn[cnt]);
                }
                if (ret) {
                        /* Back out changes we already did */
                        for (cnt--; cnt >= 0; cnt--) {
-                               if (!dquots[cnt])
+                               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+                               if (!dquot)
                                        continue;
-                               spin_lock(&dquots[cnt]->dq_dqb_lock);
+                               spin_lock(&dquot->dq_dqb_lock);
                                if (reserve)
-                                       dquot_free_reserved_space(dquots[cnt],
-                                                                 number);
+                                       dquot_free_reserved_space(dquot, number);
                                else
-                                       dquot_decr_space(dquots[cnt], number);
-                               spin_unlock(&dquots[cnt]->dq_dqb_lock);
+                                       dquot_decr_space(dquot, number);
+                               spin_unlock(&dquot->dq_dqb_lock);
                        }
                        spin_unlock(&inode->i_lock);
                        goto out_flush_warn;
@@ -1756,6 +1758,7 @@ int dquot_alloc_inode(struct inode *inode)
        int cnt, ret = 0, index;
        struct dquot_warn warn[MAXQUOTAS];
        struct dquot * const *dquots;
+       struct dquot *dquot;
 
        if (!inode_quota_active(inode))
                return 0;
@@ -1766,17 +1769,19 @@ int dquot_alloc_inode(struct inode *inode)
        index = srcu_read_lock(&dquot_srcu);
        spin_lock(&inode->i_lock);
        for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-               if (!dquots[cnt])
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (!dquot)
                        continue;
-               ret = dquot_add_inodes(dquots[cnt], 1, &warn[cnt]);
+               ret = dquot_add_inodes(dquot, 1, &warn[cnt]);
                if (ret) {
                        for (cnt--; cnt >= 0; cnt--) {
-                               if (!dquots[cnt])
+                               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+                               if (!dquot)
                                        continue;
                                /* Back out changes we already did */
-                               spin_lock(&dquots[cnt]->dq_dqb_lock);
-                               dquot_decr_inodes(dquots[cnt], 1);
-                               spin_unlock(&dquots[cnt]->dq_dqb_lock);
+                               spin_lock(&dquot->dq_dqb_lock);
+                               dquot_decr_inodes(dquot, 1);
+                               spin_unlock(&dquot->dq_dqb_lock);
                        }
                        goto warn_put_all;
                }
@@ -1798,6 +1803,7 @@ EXPORT_SYMBOL(dquot_alloc_inode);
 int dquot_claim_space_nodirty(struct inode *inode, qsize_t number)
 {
        struct dquot **dquots;
+       struct dquot *dquot;
        int cnt, index;
 
        if (!inode_quota_active(inode)) {
@@ -1813,9 +1819,8 @@ int dquot_claim_space_nodirty(struct inode *inode, qsize_t number)
        spin_lock(&inode->i_lock);
        /* Claim reserved quotas to allocated quotas */
        for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-               if (dquots[cnt]) {
-                       struct dquot *dquot = dquots[cnt];
-
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (dquot) {
                        spin_lock(&dquot->dq_dqb_lock);
                        if (WARN_ON_ONCE(dquot->dq_dqb.dqb_rsvspace < number))
                                number = dquot->dq_dqb.dqb_rsvspace;
@@ -1840,6 +1845,7 @@ EXPORT_SYMBOL(dquot_claim_space_nodirty);
 void dquot_reclaim_space_nodirty(struct inode *inode, qsize_t number)
 {
        struct dquot **dquots;
+       struct dquot *dquot;
        int cnt, index;
 
        if (!inode_quota_active(inode)) {
@@ -1855,9 +1861,8 @@ void dquot_reclaim_space_nodirty(struct inode *inode, qsize_t number)
        spin_lock(&inode->i_lock);
        /* Claim reserved quotas to allocated quotas */
        for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-               if (dquots[cnt]) {
-                       struct dquot *dquot = dquots[cnt];
-
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (dquot) {
                        spin_lock(&dquot->dq_dqb_lock);
                        if (WARN_ON_ONCE(dquot->dq_dqb.dqb_curspace < number))
                                number = dquot->dq_dqb.dqb_curspace;
@@ -1884,6 +1889,7 @@ void __dquot_free_space(struct inode *inode, qsize_t number, int flags)
        unsigned int cnt;
        struct dquot_warn warn[MAXQUOTAS];
        struct dquot **dquots;
+       struct dquot *dquot;
        int reserve = flags & DQUOT_SPACE_RESERVE, index;
 
        if (!inode_quota_active(inode)) {
@@ -1904,17 +1910,18 @@ void __dquot_free_space(struct inode *inode, qsize_t number, int flags)
                int wtype;
 
                warn[cnt].w_type = QUOTA_NL_NOWARN;
-               if (!dquots[cnt])
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (!dquot)
                        continue;
-               spin_lock(&dquots[cnt]->dq_dqb_lock);
-               wtype = info_bdq_free(dquots[cnt], number);
+               spin_lock(&dquot->dq_dqb_lock);
+               wtype = info_bdq_free(dquot, number);
                if (wtype != QUOTA_NL_NOWARN)
-                       prepare_warning(&warn[cnt], dquots[cnt], wtype);
+                       prepare_warning(&warn[cnt], dquot, wtype);
                if (reserve)
-                       dquot_free_reserved_space(dquots[cnt], number);
+                       dquot_free_reserved_space(dquot, number);
                else
-                       dquot_decr_space(dquots[cnt], number);
-               spin_unlock(&dquots[cnt]->dq_dqb_lock);
+                       dquot_decr_space(dquot, number);
+               spin_unlock(&dquot->dq_dqb_lock);
        }
        if (reserve)
                *inode_reserved_space(inode) -= number;
@@ -1939,6 +1946,7 @@ void dquot_free_inode(struct inode *inode)
        unsigned int cnt;
        struct dquot_warn warn[MAXQUOTAS];
        struct dquot * const *dquots;
+       struct dquot *dquot;
        int index;
 
        if (!inode_quota_active(inode))
@@ -1949,16 +1957,16 @@ void dquot_free_inode(struct inode *inode)
        spin_lock(&inode->i_lock);
        for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
                int wtype;
-
                warn[cnt].w_type = QUOTA_NL_NOWARN;
-               if (!dquots[cnt])
+               dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
+               if (!dquot)
                        continue;
-               spin_lock(&dquots[cnt]->dq_dqb_lock);
-               wtype = info_idq_free(dquots[cnt], 1);
+               spin_lock(&dquot->dq_dqb_lock);
+               wtype = info_idq_free(dquot, 1);
                if (wtype != QUOTA_NL_NOWARN)
-                       prepare_warning(&warn[cnt], dquots[cnt], wtype);
-               dquot_decr_inodes(dquots[cnt], 1);
-               spin_unlock(&dquots[cnt]->dq_dqb_lock);
+                       prepare_warning(&warn[cnt], dquot, wtype);
+               dquot_decr_inodes(dquot, 1);
+               spin_unlock(&dquot->dq_dqb_lock);
        }
        spin_unlock(&inode->i_lock);
        mark_all_dquot_dirty(dquots);
@@ -1985,7 +1993,7 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
        qsize_t rsv_space = 0;
        qsize_t inode_usage = 1;
        struct dquot *transfer_from[MAXQUOTAS] = {};
-       int cnt, ret = 0;
+       int cnt, index, ret = 0;
        char is_valid[MAXQUOTAS] = {};
        struct dquot_warn warn_to[MAXQUOTAS];
        struct dquot_warn warn_from_inodes[MAXQUOTAS];
@@ -2074,8 +2082,16 @@ int __dquot_transfer(struct inode *inode, struct dquot **transfer_to)
        spin_unlock(&inode->i_lock);
        spin_unlock(&dq_data_lock);
 
+       /*
+        * These arrays are local and we hold dquot references so we don't need
+        * the srcu protection but still take dquot_srcu to avoid warning in
+        * mark_all_dquot_dirty().
+        */
+       index = srcu_read_lock(&dquot_srcu);
        mark_all_dquot_dirty(transfer_from);
        mark_all_dquot_dirty(transfer_to);
+       srcu_read_unlock(&dquot_srcu, index);
+
        flush_warnings(warn_to);
        flush_warnings(warn_from_inodes);
        flush_warnings(warn_from_space);