From: Jan Kara <jack@suse.cz>
Fix possible races between umount and quota on/off.
Finally I decided to take a reference to vfsmount during vfs_quota_on() and
to drop it after the final cleanup in the vfs_quota_off(). This way we
should be all the time guarded against umount. This way was protected also
the old code which used filp_open() for opening quota files. I was also
thinking about other ways of protection but there would be always a window
(provided I don't want to play much with namespace locks) where
vfs_quota_on() could be called while umount() is in progress resulting in
the "Busy inodes after unmount" messages...
Get a reference to vfsmount during quotaon() so that we are guarded against
umount (as was the old code using filp_open()).
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
25-akpm/fs/dquot.c | 45 ++++++++++++++++++++++++++++-----------
25-akpm/include/linux/quota.h | 1
25-akpm/include/linux/quotaops.h | 2 -
3 files changed, 35 insertions(+), 13 deletions(-)
diff -puN fs/dquot.c~quota-umount-race-fix fs/dquot.c
--- 25/fs/dquot.c~quota-umount-race-fix Tue Nov 23 17:11:34 2004
+++ 25-akpm/fs/dquot.c Tue Nov 23 17:11:34 2004
@@ -1314,12 +1314,14 @@ int vfs_quota_off(struct super_block *sb
{
int cnt;
struct quota_info *dqopt = sb_dqopt(sb);
- struct inode *toput[MAXQUOTAS];
+ struct inode *toputinode[MAXQUOTAS];
+ struct vfsmount *toputmnt[MAXQUOTAS];
/* We need to serialize quota_off() for device */
down(&dqopt->dqonoff_sem);
for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
- toput[cnt] = NULL;
+ toputinode[cnt] = NULL;
+ toputmnt[cnt] = NULL;
if (type != -1 && cnt != type)
continue;
if (!sb_has_quota_enabled(sb, cnt))
@@ -1339,8 +1341,10 @@ int vfs_quota_off(struct super_block *sb
dqopt->ops[cnt]->free_file_info(sb, cnt);
put_quota_format(dqopt->info[cnt].dqi_format);
- toput[cnt] = dqopt->files[cnt];
+ toputinode[cnt] = dqopt->files[cnt];
+ toputmnt[cnt] = dqopt->mnt[cnt];
dqopt->files[cnt] = NULL;
+ dqopt->mnt[cnt] = NULL;
dqopt->info[cnt].dqi_flags = 0;
dqopt->info[cnt].dqi_igrace = 0;
dqopt->info[cnt].dqi_bgrace = 0;
@@ -1348,7 +1352,10 @@ int vfs_quota_off(struct super_block *sb
}
up(&dqopt->dqonoff_sem);
/* Sync the superblock so that buffers with quota data are written to
- * disk (and so userspace sees correct data afterwards) */
+ * disk (and so userspace sees correct data afterwards).
+ * The reference to vfsmnt we are still holding protects us from
+ * umount (we don't have it only when quotas are turned on/off for
+ * journal replay but in that case we are guarded by the fs anyway). */
if (sb->s_op->sync_fs)
sb->s_op->sync_fs(sb, 1);
sync_blockdev(sb->s_bdev);
@@ -1358,13 +1365,24 @@ int vfs_quota_off(struct super_block *sb
* must also discard the blockdev buffers so that we see the
* changes done by userspace on the next quotaon() */
for (cnt = 0; cnt < MAXQUOTAS; cnt++)
- if (toput[cnt]) {
- down(&toput[cnt]->i_sem);
- toput[cnt]->i_flags &= ~(S_IMMUTABLE | S_NOATIME | S_NOQUOTA);
- truncate_inode_pages(&toput[cnt]->i_data, 0);
- up(&toput[cnt]->i_sem);
- mark_inode_dirty(toput[cnt]);
- iput(toput[cnt]);
+ if (toputinode[cnt]) {
+ down(&dqopt->dqonoff_sem);
+ /* If quota was reenabled in the meantime, we have
+ * nothing to do */
+ if (!sb_has_quota_enabled(sb, cnt)) {
+ down(&toputinode[cnt]->i_sem);
+ toputinode[cnt]->i_flags &= ~(S_IMMUTABLE |
+ S_NOATIME | S_NOQUOTA);
+ truncate_inode_pages(&toputinode[cnt]->i_data, 0);
+ up(&toputinode[cnt]->i_sem);
+ mark_inode_dirty(toputinode[cnt]);
+ iput(toputinode[cnt]);
+ }
+ up(&dqopt->dqonoff_sem);
+ /* We don't hold the reference when we turned on quotas
+ * just for the journal replay... */
+ if (toputmnt[cnt])
+ mntput(toputmnt[cnt]);
}
if (sb->s_bdev)
invalidate_bdev(sb->s_bdev, 0);
@@ -1478,8 +1496,11 @@ int vfs_quota_on(struct super_block *sb,
/* Quota file not on the same filesystem? */
if (nd.mnt->mnt_sb != sb)
error = -EXDEV;
- else
+ else {
error = vfs_quota_on_inode(nd.dentry->d_inode, type, format_id);
+ if (!error)
+ sb_dqopt(sb)->mnt[type] = mntget(nd.mnt);
+ }
out_path:
path_release(&nd);
return error;
diff -puN include/linux/quota.h~quota-umount-race-fix include/linux/quota.h
--- 25/include/linux/quota.h~quota-umount-race-fix Tue Nov 23 17:11:34 2004
+++ 25-akpm/include/linux/quota.h Tue Nov 23 17:11:34 2004
@@ -286,6 +286,7 @@ struct quota_info {
struct semaphore dqonoff_sem; /* Serialize quotaon & quotaoff */
struct rw_semaphore dqptr_sem; /* serialize ops using quota_info struct, pointers from inode to dquots */
struct inode *files[MAXQUOTAS]; /* inodes of quotafiles */
+ struct vfsmount *mnt[MAXQUOTAS]; /* mountpoint entries of filesystems with quota files */
struct mem_dqinfo info[MAXQUOTAS]; /* Information for each quota type */
struct quota_format_ops *ops[MAXQUOTAS]; /* Operations for each type */
};
diff -puN include/linux/quotaops.h~quota-umount-race-fix include/linux/quotaops.h
--- 25/include/linux/quotaops.h~quota-umount-race-fix Tue Nov 23 17:11:34 2004
+++ 25-akpm/include/linux/quotaops.h Tue Nov 23 17:11:34 2004
@@ -177,7 +177,7 @@ static __inline__ int DQUOT_OFF(struct s
{
int ret = -ENOSYS;
- if (sb->s_qcop && sb->s_qcop->quota_off)
+ if (sb_any_quota_enabled(sb) && sb->s_qcop && sb->s_qcop->quota_off)
ret = sb->s_qcop->quota_off(sb, -1);
return ret;
}
_