* [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock()
@ 2015-04-07 17:48 Abhi Das
2015-04-08 10:01 ` Steven Whitehouse
2015-04-08 14:38 ` Bob Peterson
0 siblings, 2 replies; 3+ messages in thread
From: Abhi Das @ 2015-04-07 17:48 UTC (permalink / raw)
To: cluster-devel.redhat.com
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 <adas@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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock()
2015-04-07 17:48 [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock() Abhi Das
@ 2015-04-08 10:01 ` Steven Whitehouse
2015-04-08 14:38 ` Bob Peterson
1 sibling, 0 replies; 3+ messages in thread
From: Steven Whitehouse @ 2015-04-08 10:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
That makes sense to me. Acked-by: Steven Whitehouse <swhiteho@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 <adas@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;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock()
2015-04-07 17:48 [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock() Abhi Das
2015-04-08 10:01 ` Steven Whitehouse
@ 2015-04-08 14:38 ` Bob Peterson
1 sibling, 0 replies; 3+ messages in thread
From: Bob Peterson @ 2015-04-08 14:38 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- 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 <adas@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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-04-08 14:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-07 17:48 [Cluster-devel] [GFS2] gfs2: fix quota refresh race in do_glock() Abhi Das
2015-04-08 10:01 ` Steven Whitehouse
2015-04-08 14:38 ` Bob Peterson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.