Re: [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock()

2015-04-08 Thread Steven Whitehouse 

Hi,

That makes sense to me. Acked-by: Steven Whitehouse swhit...@redhat.com

Steve.

On 07/04/15 18:48, Abhi Das wrote:

quotad periodically syncs in-memory quotas to the ondisk quota file
and sets the QDF_REFRESH flag so that a subsequent read of a synced
quota is re-read from disk.

gfs2_quota_lock() checks for this flag and sets a 'force' bit to
force re-read from disk if requested. However, there is a race
condition here. It is possible for gfs2_quota_lock() to find the
QDF_REFRESH flag unset (i.e force=0) and quotad comes in immediately
after and syncs the relevant quota and sets the QDF_REFRESH flag.
gfs2_quota_lock() resumes with force=0 and uses the stale in-memory
quota usage values that result in miscalculations.

This patch fixes this race by moving the check for the QDF_REFRESH
flag check further out into the gfs2_quota_lock() process, i.e, in
do_glock(), under the protection of the quota glock.

Resolves: rhbz#1174295
Signed-off-by: Abhi Das a...@redhat.com
---
  fs/gfs2/quota.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 5561468..5c27e48 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -923,6 +923,9 @@ restart:
if (error)
return error;
  
+	if (test_and_clear_bit(QDF_REFRESH, qd-qd_flags))

+   force_refresh = FORCE;
+
qd-qd_qb = *(struct gfs2_quota_lvb *)qd-qd_gl-gl_lksb.sb_lvbptr;
  
  	if (force_refresh || qd-qd_qb.qb_magic != cpu_to_be32(GFS2_MAGIC)) {

@@ -974,11 +977,8 @@ int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, 
kgid_t gid)
 sizeof(struct gfs2_quota_data *), sort_qd, NULL);
  
  	for (x = 0; x  ip-i_res-rs_qa_qd_num; x++) {

-   int force = NO_FORCE;
qd = ip-i_res-rs_qa_qd[x];
-   if (test_and_clear_bit(QDF_REFRESH, qd-qd_flags))
-   force = FORCE;
-   error = do_glock(qd, force, ip-i_res-rs_qa_qd_ghs[x]);
+   error = do_glock(qd, NO_FORCE, ip-i_res-rs_qa_qd_ghs[x]);
if (error)
break;
}

Re: [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock()

2015-04-08 Thread Bob Peterson 
- Original Message -
 quotad periodically syncs in-memory quotas to the ondisk quota file
 and sets the QDF_REFRESH flag so that a subsequent read of a synced
 quota is re-read from disk.
 
 gfs2_quota_lock() checks for this flag and sets a 'force' bit to
 force re-read from disk if requested. However, there is a race
 condition here. It is possible for gfs2_quota_lock() to find the
 QDF_REFRESH flag unset (i.e force=0) and quotad comes in immediately
 after and syncs the relevant quota and sets the QDF_REFRESH flag.
 gfs2_quota_lock() resumes with force=0 and uses the stale in-memory
 quota usage values that result in miscalculations.
 
 This patch fixes this race by moving the check for the QDF_REFRESH
 flag check further out into the gfs2_quota_lock() process, i.e, in
 do_glock(), under the protection of the quota glock.
 
 Resolves: rhbz#1174295
 Signed-off-by: Abhi Das a...@redhat.com
 ---

Hi,

ACK

I pushed this patch to the for-next branch of the linux-gfs2 tree.

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock()

2015-04-07 Thread Abhi Das 
quotad periodically syncs in-memory quotas to the ondisk quota file
and sets the QDF_REFRESH flag so that a subsequent read of a synced
quota is re-read from disk.

gfs2_quota_lock() checks for this flag and sets a 'force' bit to
force re-read from disk if requested. However, there is a race
condition here. It is possible for gfs2_quota_lock() to find the
QDF_REFRESH flag unset (i.e force=0) and quotad comes in immediately
after and syncs the relevant quota and sets the QDF_REFRESH flag.
gfs2_quota_lock() resumes with force=0 and uses the stale in-memory
quota usage values that result in miscalculations.

This patch fixes this race by moving the check for the QDF_REFRESH
flag check further out into the gfs2_quota_lock() process, i.e, in
do_glock(), under the protection of the quota glock.

Resolves: rhbz#1174295
Signed-off-by: Abhi Das a...@redhat.com
---
 fs/gfs2/quota.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 5561468..5c27e48 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -923,6 +923,9 @@ restart:
if (error)
return error;
 
+   if (test_and_clear_bit(QDF_REFRESH, qd-qd_flags))
+   force_refresh = FORCE;
+
qd-qd_qb = *(struct gfs2_quota_lvb *)qd-qd_gl-gl_lksb.sb_lvbptr;
 
if (force_refresh || qd-qd_qb.qb_magic != cpu_to_be32(GFS2_MAGIC)) {
@@ -974,11 +977,8 @@ int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, 
kgid_t gid)
 sizeof(struct gfs2_quota_data *), sort_qd, NULL);
 
for (x = 0; x  ip-i_res-rs_qa_qd_num; x++) {
-   int force = NO_FORCE;
qd = ip-i_res-rs_qa_qd[x];
-   if (test_and_clear_bit(QDF_REFRESH, qd-qd_flags))
-   force = FORCE;
-   error = do_glock(qd, force, ip-i_res-rs_qa_qd_ghs[x]);
+   error = do_glock(qd, NO_FORCE, ip-i_res-rs_qa_qd_ghs[x]);
if (error)
break;
}
-- 
1.8.1.4