gfs2: Change inode qa_data to allow multiple users
authorBob Peterson <rpeterso@redhat.com>
Thu, 27 Feb 2020 18:47:53 +0000 (12:47 -0600)
committerBob Peterson <rpeterso@redhat.com>
Fri, 27 Mar 2020 19:08:04 +0000 (14:08 -0500)
Before this patch, multiple users called gfs2_qa_alloc which allocated
a qadata structure to the inode, if quotas are turned on. Later, in
file close or evict, the structure was deleted with gfs2_qa_delete.
But there can be several competing processes who need access to the
structure. There were races between file close (release) and the others.
Thus, a release could delete the structure out from under a process
that relied upon its existence. For example, chown.

This patch changes the management of the qadata structures to be
a get/put scheme. Function gfs2_qa_alloc has been changed to gfs2_qa_get
and if the structure is allocated, the count essentially starts out at
1. Function gfs2_qa_delete has been renamed to gfs2_qa_put, and the
last guy to decrement the count to 0 frees the memory.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
fs/gfs2/acl.c
fs/gfs2/bmap.c
fs/gfs2/file.c
fs/gfs2/incore.h
fs/gfs2/inode.c
fs/gfs2/quota.c
fs/gfs2/quota.h
fs/gfs2/rgrp.c
fs/gfs2/super.c
fs/gfs2/xattr.c

index cb09b85c5b107f9e69f84cdd2c59279bc8d4ad6f..2e939f5fe751cde6fda5515948af6c3a4983145e 100644 (file)
@@ -117,14 +117,14 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl *acl, int type)
        if (acl && acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode)))
                return -E2BIG;
 
-       ret = gfs2_qa_alloc(ip);
+       ret = gfs2_qa_get(ip);
        if (ret)
                return ret;
 
        if (!gfs2_glock_is_locked_by_me(ip->i_gl)) {
                ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
                if (ret)
-                       return ret;
+                       goto out;
                need_unlock = true;
        }
 
@@ -144,5 +144,7 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 unlock:
        if (need_unlock)
                gfs2_glock_dq_uninit(&gh);
+out:
+       gfs2_qa_put(ip);
        return ret;
 }
index 4b9dbab50faff92b137fd4294d417215c5b0785f..d510a453dfa8844525f762371f5ba94f20137842 100644 (file)
@@ -2183,7 +2183,7 @@ int gfs2_setattr_size(struct inode *inode, u64 newsize)
 
        inode_dio_wait(inode);
 
-       ret = gfs2_qa_alloc(ip);
+       ret = gfs2_qa_get(ip);
        if (ret)
                goto out;
 
index 54b0708e6d353e188be0df0ac93d2d0ffaa13de8..f18876cdfb0f5f98559f23121bfc1790ce737421 100644 (file)
@@ -458,7 +458,7 @@ static vm_fault_t gfs2_page_mkwrite(struct vm_fault *vmf)
 
        sb_start_pagefault(inode->i_sb);
 
-       ret = gfs2_qa_alloc(ip);
+       ret = gfs2_qa_get(ip);
        if (ret)
                goto out;
 
@@ -553,6 +553,7 @@ out_quota_unlock:
 out_unlock:
        gfs2_glock_dq(&gh);
 out_uninit:
+       gfs2_qa_put(ip);
        gfs2_holder_uninit(&gh);
        if (ret == 0) {
                set_page_dirty(page);
@@ -635,7 +636,17 @@ int gfs2_open_common(struct inode *inode, struct file *file)
 
        gfs2_assert_warn(GFS2_SB(inode), !file->private_data);
        file->private_data = fp;
+       if (file->f_mode & FMODE_WRITE) {
+               ret = gfs2_qa_get(GFS2_I(inode));
+               if (ret)
+                       goto fail;
+       }
        return 0;
+
+fail:
+       kfree(file->private_data);
+       file->private_data = NULL;
+       return ret;
 }
 
 /**
@@ -690,10 +701,8 @@ static int gfs2_release(struct inode *inode, struct file *file)
        kfree(file->private_data);
        file->private_data = NULL;
 
-       if (!(file->f_mode & FMODE_WRITE))
-               return 0;
-
-       gfs2_rsqa_delete(ip, &inode->i_writecount);
+       if (file->f_mode & FMODE_WRITE)
+               gfs2_rsqa_delete(ip, &inode->i_writecount);
        return 0;
 }
 
@@ -849,7 +858,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
        struct gfs2_inode *ip = GFS2_I(inode);
        ssize_t ret;
 
-       ret = gfs2_qa_alloc(ip);
+       ret = gfs2_qa_get(ip);
        if (ret)
                return ret;
 
@@ -860,7 +869,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
                ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
                if (ret)
-                       return ret;
+                       goto out;
                gfs2_glock_dq_uninit(&gh);
        }
 
@@ -918,6 +927,8 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 out_unlock:
        inode_unlock(inode);
+out:
+       gfs2_qa_put(ip);
        return ret;
 }
 
@@ -1149,7 +1160,7 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
        if (mode & FALLOC_FL_PUNCH_HOLE) {
                ret = __gfs2_punch_hole(file, offset, len);
        } else {
-               ret = gfs2_qa_alloc(ip);
+               ret = gfs2_qa_get(ip);
                if (ret)
                        goto out_putw;
 
@@ -1157,6 +1168,7 @@ static long gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 
                if (ret)
                        gfs2_rs_deltree(&ip->i_res);
+               gfs2_qa_put(ip);
        }
 
 out_putw:
@@ -1175,14 +1187,17 @@ static ssize_t gfs2_file_splice_write(struct pipe_inode_info *pipe,
 {
        int error;
        struct gfs2_inode *ip = GFS2_I(out->f_mapping->host);
+       ssize_t ret;
 
-       error = gfs2_qa_alloc(ip);
+       error = gfs2_qa_get(ip);
        if (error)
                return (ssize_t)error;
 
        gfs2_size_hint(out, *ppos, len);
 
-       return iter_file_splice_write(pipe, out, ppos, len, flags);
+       ret = iter_file_splice_write(pipe, out, ppos, len, flags);
+       gfs2_qa_put(ip);
+       return ret;
 }
 
 #ifdef CONFIG_GFS2_FS_LOCKING_DLM
index 04549a8cae7e61d7eef67f8ff4421883889a6150..84a824293a78ba70cfa89f9c58e4981f3c71d032 100644 (file)
@@ -295,6 +295,7 @@ struct gfs2_qadata { /* quota allocation data */
        struct gfs2_quota_data *qa_qd[2 * GFS2_MAXQUOTAS];
        struct gfs2_holder qa_qd_ghs[2 * GFS2_MAXQUOTAS];
        unsigned int qa_qd_num;
+       int qa_ref;
 };
 
 /* Resource group multi-block reservation, in order of appearance:
index 710f1c644f8733691930a51d0c7c429fed757efa..d1a24753c55f3e269148e480e83adbf2a3f64b63 100644 (file)
@@ -588,13 +588,13 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
        if (!name->len || name->len > GFS2_FNAMESIZE)
                return -ENAMETOOLONG;
 
-       error = gfs2_qa_alloc(dip);
+       error = gfs2_qa_get(dip);
        if (error)
                return error;
 
        error = gfs2_rindex_update(sdp);
        if (error)
-               return error;
+               goto fail;
 
        error = gfs2_glock_nq_init(dip->i_gl, LM_ST_EXCLUSIVE, 0, ghs);
        if (error)
@@ -641,7 +641,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
                goto fail_gunlock;
 
        ip = GFS2_I(inode);
-       error = gfs2_qa_alloc(ip);
+       error = gfs2_qa_get(ip);
        if (error)
                goto fail_free_acls;
 
@@ -776,6 +776,7 @@ fail_gunlock2:
        clear_bit(GLF_INODE_CREATING, &io_gl->gl_flags);
        gfs2_glock_put(io_gl);
 fail_free_inode:
+       gfs2_qa_put(ip);
        if (ip->i_gl) {
                glock_clear_object(ip->i_gl, ip);
                gfs2_glock_put(ip->i_gl);
@@ -798,6 +799,7 @@ fail_gunlock:
        if (gfs2_holder_initialized(ghs + 1))
                gfs2_glock_dq_uninit(ghs + 1);
 fail:
+       gfs2_qa_put(dip);
        return error;
 }
 
@@ -899,7 +901,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
        if (S_ISDIR(inode->i_mode))
                return -EPERM;
 
-       error = gfs2_qa_alloc(dip);
+       error = gfs2_qa_get(dip);
        if (error)
                return error;
 
@@ -1002,6 +1004,7 @@ out_gunlock:
 out_child:
        gfs2_glock_dq(ghs);
 out_parent:
+       gfs2_qa_put(ip);
        gfs2_holder_uninit(ghs);
        gfs2_holder_uninit(ghs + 1);
        return error;
@@ -1362,7 +1365,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
        if (error)
                return error;
 
-       error = gfs2_qa_alloc(ndip);
+       error = gfs2_qa_get(ndip);
        if (error)
                return error;
 
@@ -1562,6 +1565,7 @@ out_gunlock_r:
        if (gfs2_holder_initialized(&r_gh))
                gfs2_glock_dq_uninit(&r_gh);
 out:
+       gfs2_qa_put(ndip);
        return error;
 }
 
@@ -1873,10 +1877,9 @@ static int setattr_chown(struct inode *inode, struct iattr *attr)
                ouid = nuid = NO_UID_QUOTA_CHANGE;
        if (!(attr->ia_valid & ATTR_GID) || gid_eq(ogid, ngid))
                ogid = ngid = NO_GID_QUOTA_CHANGE;
-
-       error = gfs2_qa_alloc(ip);
+       error = gfs2_qa_get(ip);
        if (error)
-               goto out;
+               return error;
 
        error = gfs2_rindex_update(sdp);
        if (error)
@@ -1914,6 +1917,7 @@ out_end_trans:
 out_gunlock_q:
        gfs2_quota_unlock(ip);
 out:
+       gfs2_qa_put(ip);
        return error;
 }
 
@@ -1935,21 +1939,21 @@ static int gfs2_setattr(struct dentry *dentry, struct iattr *attr)
        struct gfs2_holder i_gh;
        int error;
 
-       error = gfs2_qa_alloc(ip);
+       error = gfs2_qa_get(ip);
        if (error)
                return error;
 
        error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &i_gh);
        if (error)
-               return error;
+               goto out;
 
        error = -EPERM;
        if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
-               goto out;
+               goto error;
 
        error = setattr_prepare(dentry, attr);
        if (error)
-               goto out;
+               goto error;
 
        if (attr->ia_valid & ATTR_SIZE)
                error = gfs2_setattr_size(inode, attr->ia_size);
@@ -1961,10 +1965,12 @@ static int gfs2_setattr(struct dentry *dentry, struct iattr *attr)
                        error = posix_acl_chmod(inode, inode->i_mode);
        }
 
-out:
+error:
        if (!error)
                mark_inode_dirty(inode);
        gfs2_glock_dq_uninit(&i_gh);
+out:
+       gfs2_qa_put(ip);
        return error;
 }
 
index cbe45e8eb2e0b049a5efa9d94a8e8bd76dfce610..cc0c4b5800be93b658b82bbb4b4eab09bbece428 100644 (file)
@@ -525,11 +525,11 @@ static void qdsb_put(struct gfs2_quota_data *qd)
 }
 
 /**
- * gfs2_qa_alloc - make sure we have a quota allocations data structure,
- *                 if necessary
+ * gfs2_qa_get - make sure we have a quota allocations data structure,
+ *               if necessary
  * @ip: the inode for this reservation
  */
-int gfs2_qa_alloc(struct gfs2_inode *ip)
+int gfs2_qa_get(struct gfs2_inode *ip)
 {
        int error = 0;
        struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
@@ -540,17 +540,21 @@ int gfs2_qa_alloc(struct gfs2_inode *ip)
        down_write(&ip->i_rw_mutex);
        if (ip->i_qadata == NULL) {
                ip->i_qadata = kmem_cache_zalloc(gfs2_qadata_cachep, GFP_NOFS);
-               if (!ip->i_qadata)
+               if (!ip->i_qadata) {
                        error = -ENOMEM;
+                       goto out;
+               }
        }
+       ip->i_qadata->qa_ref++;
+out:
        up_write(&ip->i_rw_mutex);
        return error;
 }
 
-void gfs2_qa_delete(struct gfs2_inode *ip, atomic_t *wcount)
+void gfs2_qa_put(struct gfs2_inode *ip)
 {
        down_write(&ip->i_rw_mutex);
-       if (ip->i_qadata && ((wcount == NULL) || (atomic_read(wcount) <= 1))) {
+       if (ip->i_qadata && --ip->i_qadata->qa_ref == 0) {
                kmem_cache_free(gfs2_qadata_cachep, ip->i_qadata);
                ip->i_qadata = NULL;
        }
@@ -566,27 +570,27 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
        if (sdp->sd_args.ar_quota == GFS2_QUOTA_OFF)
                return 0;
 
-       if (ip->i_qadata == NULL) {
-               error = gfs2_qa_alloc(ip);
-               if (error)
-                       return error;
-       }
+       error = gfs2_qa_get(ip);
+       if (error)
+               return error;
 
        qd = ip->i_qadata->qa_qd;
 
        if (gfs2_assert_warn(sdp, !ip->i_qadata->qa_qd_num) ||
-           gfs2_assert_warn(sdp, !test_bit(GIF_QD_LOCKED, &ip->i_flags)))
-               return -EIO;
+           gfs2_assert_warn(sdp, !test_bit(GIF_QD_LOCKED, &ip->i_flags))) {
+               error = -EIO;
+               goto out;
+       }
 
        error = qdsb_get(sdp, make_kqid_uid(ip->i_inode.i_uid), qd);
        if (error)
-               goto out;
+               goto out_unhold;
        ip->i_qadata->qa_qd_num++;
        qd++;
 
        error = qdsb_get(sdp, make_kqid_gid(ip->i_inode.i_gid), qd);
        if (error)
-               goto out;
+               goto out_unhold;
        ip->i_qadata->qa_qd_num++;
        qd++;
 
@@ -594,7 +598,7 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
            !uid_eq(uid, ip->i_inode.i_uid)) {
                error = qdsb_get(sdp, make_kqid_uid(uid), qd);
                if (error)
-                       goto out;
+                       goto out_unhold;
                ip->i_qadata->qa_qd_num++;
                qd++;
        }
@@ -603,14 +607,15 @@ int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
            !gid_eq(gid, ip->i_inode.i_gid)) {
                error = qdsb_get(sdp, make_kqid_gid(gid), qd);
                if (error)
-                       goto out;
+                       goto out_unhold;
                ip->i_qadata->qa_qd_num++;
                qd++;
        }
 
-out:
+out_unhold:
        if (error)
                gfs2_quota_unhold(ip);
+out:
        return error;
 }
 
@@ -621,6 +626,7 @@ void gfs2_quota_unhold(struct gfs2_inode *ip)
 
        if (ip->i_qadata == NULL)
                return;
+
        gfs2_assert_warn(sdp, !test_bit(GIF_QD_LOCKED, &ip->i_flags));
 
        for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
@@ -628,6 +634,7 @@ void gfs2_quota_unhold(struct gfs2_inode *ip)
                ip->i_qadata->qa_qd[x] = NULL;
        }
        ip->i_qadata->qa_qd_num = 0;
+       gfs2_qa_put(ip);
 }
 
 static int sort_qd(const void *a, const void *b)
@@ -876,7 +883,7 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
        unsigned int nalloc = 0, blocks;
        int error;
 
-       error = gfs2_qa_alloc(ip);
+       error = gfs2_qa_get(ip);
        if (error)
                return error;
 
@@ -884,8 +891,10 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
                              &data_blocks, &ind_blocks);
 
        ghs = kmalloc_array(num_qd, sizeof(struct gfs2_holder), GFP_NOFS);
-       if (!ghs)
-               return -ENOMEM;
+       if (!ghs) {
+               error = -ENOMEM;
+               goto out;
+       }
 
        sort(qda, num_qd, sizeof(struct gfs2_quota_data *), sort_qd, NULL);
        inode_lock(&ip->i_inode);
@@ -893,12 +902,12 @@ static int do_sync(unsigned int num_qd, struct gfs2_quota_data **qda)
                error = gfs2_glock_nq_init(qda[qx]->qd_gl, LM_ST_EXCLUSIVE,
                                           GL_NOCACHE, &ghs[qx]);
                if (error)
-                       goto out;
+                       goto out_dq;
        }
 
        error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &i_gh);
        if (error)
-               goto out;
+               goto out_dq;
 
        for (x = 0; x < num_qd; x++) {
                offset = qd2offset(qda[x]);
@@ -950,13 +959,15 @@ out_ipres:
        gfs2_inplace_release(ip);
 out_alloc:
        gfs2_glock_dq_uninit(&i_gh);
-out:
+out_dq:
        while (qx--)
                gfs2_glock_dq_uninit(&ghs[qx]);
        inode_unlock(&ip->i_inode);
        kfree(ghs);
        gfs2_log_flush(ip->i_gl->gl_name.ln_sbd, ip->i_gl,
                       GFS2_LOG_HEAD_FLUSH_NORMAL | GFS2_LFC_DO_SYNC);
+out:
+       gfs2_qa_put(ip);
        return error;
 }
 
@@ -1259,6 +1270,7 @@ void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
        if (ip->i_diskflags & GFS2_DIF_SYSTEM)
                return;
 
+       BUG_ON(ip->i_qadata->qa_ref <= 0);
        for (x = 0; x < ip->i_qadata->qa_qd_num; x++) {
                qd = ip->i_qadata->qa_qd[x];
 
@@ -1677,7 +1689,7 @@ static int gfs2_set_dqblk(struct super_block *sb, struct kqid qid,
        if (error)
                return error;
 
-       error = gfs2_qa_alloc(ip);
+       error = gfs2_qa_get(ip);
        if (error)
                goto out_put;
 
@@ -1746,6 +1758,7 @@ out_i:
 out_q:
        gfs2_glock_dq_uninit(&q_gh);
 out_unlockput:
+       gfs2_qa_put(ip);
        inode_unlock(&ip->i_inode);
 out_put:
        qd_put(qd);
index 765627d9a91ec24c4d20d3408d494e7a84866e1a..7f9ca8ef40fc4867454d97d37f8072e36dc1dbaa 100644 (file)
@@ -15,8 +15,8 @@ struct gfs2_sbd;
 #define NO_UID_QUOTA_CHANGE INVALID_UID
 #define NO_GID_QUOTA_CHANGE INVALID_GID
 
-extern int gfs2_qa_alloc(struct gfs2_inode *ip);
-extern void gfs2_qa_delete(struct gfs2_inode *ip, atomic_t *wcount);
+extern int gfs2_qa_get(struct gfs2_inode *ip);
+extern void gfs2_qa_put(struct gfs2_inode *ip);
 extern int gfs2_quota_hold(struct gfs2_inode *ip, kuid_t uid, kgid_t gid);
 extern void gfs2_quota_unhold(struct gfs2_inode *ip);
 
index 3e3696da5bcb95bccc6764363cb23659483fbb96..04e3e13a230c1118adccbe1cfbc60a352ae564e4 100644 (file)
@@ -673,7 +673,7 @@ void gfs2_rsqa_delete(struct gfs2_inode *ip, atomic_t *wcount)
        if ((wcount == NULL) || (atomic_read(wcount) <= 1))
                gfs2_rs_deltree(&ip->i_res);
        up_write(&ip->i_rw_mutex);
-       gfs2_qa_delete(ip, wcount);
+       gfs2_qa_put(ip);
 }
 
 /**
index a6bf8f1e083e6fc898132c69319ece215ccdd966..68d934fa0f9fb6a43b2704cf9c6a02a19970ca83 100644 (file)
@@ -1401,6 +1401,8 @@ out_unlock:
                fs_warn(sdp, "gfs2_evict_inode: %d\n", error);
 out:
        truncate_inode_pages_final(&inode->i_data);
+       if (ip->i_qadata)
+               gfs2_assert_warn(sdp, ip->i_qadata->qa_ref == 0);
        gfs2_rsqa_delete(ip, NULL);
        gfs2_ordered_del_inode(ip);
        clear_inode(inode);
index c4fbb96e001ffabf9013cb4fad154fec1fe77e61..9d7667bc429278bc742a4511db6785542334533c 100644 (file)
@@ -1222,7 +1222,7 @@ static int gfs2_xattr_set(const struct xattr_handler *handler,
        struct gfs2_holder gh;
        int ret;
 
-       ret = gfs2_qa_alloc(ip);
+       ret = gfs2_qa_get(ip);
        if (ret)
                return ret;
 
@@ -1231,15 +1231,19 @@ static int gfs2_xattr_set(const struct xattr_handler *handler,
        if (!gfs2_glock_is_locked_by_me(ip->i_gl)) {
                ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
                if (ret)
-                       return ret;
+                       goto out;
        } else {
-               if (WARN_ON_ONCE(ip->i_gl->gl_state != LM_ST_EXCLUSIVE))
-                       return -EIO;
+               if (WARN_ON_ONCE(ip->i_gl->gl_state != LM_ST_EXCLUSIVE)) {
+                       ret = -EIO;
+                       goto out;
+               }
                gfs2_holder_mark_uninitialized(&gh);
        }
        ret = __gfs2_xattr_set(inode, name, value, size, flags, handler->flags);
        if (gfs2_holder_initialized(&gh))
                gfs2_glock_dq_uninit(&gh);
+out:
+       gfs2_qa_put(ip);
        return ret;
 }