]>
Commit | Line | Data |
---|---|---|
00e5a55c BS |
1 | From: Jan Kara <jack@suse.cz> |
2 | Subject: ocfs2: Fix possible deadlock with quotas in ocfs2_setattr() | |
3 | Patch-mainline: 2.6.31 | |
4 | ||
5 | We called vfs_dq_transfer() with global quota file lock held. This can lead | |
6 | to deadlocks as if vfs_dq_transfer() has to allocate new quota structure, | |
7 | it calls ocfs2_dquot_acquire() which tries to get quota file lock again and | |
8 | this can block if another node requested the lock in the mean time. | |
9 | ||
10 | Since we have to call vfs_dq_transfer() with transaction already started | |
11 | and quota file lock ranks above the transaction start, we cannot just rely | |
12 | on ocfs2_dquot_acquire() or ocfs2_dquot_release() on getting the lock | |
13 | if they need it. We fix the problem by acquiring pointers to all quota | |
14 | structures needed by vfs_dq_transfer() already before calling the function. | |
15 | By this we are sure that all quota structures are properly allocated and | |
16 | they can be freed only after we drop references to them. Thus we don't need | |
17 | quota file lock anywhere inside vfs_dq_transfer(). | |
18 | ||
19 | Signed-off-by: Jan Kara <jack@suse.cz> | |
20 | --- | |
21 | fs/ocfs2/file.c | 53 ++++++++++++++++++++++++++++++----------------------- | |
22 | 1 files changed, 30 insertions(+), 23 deletions(-) | |
23 | ||
24 | diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c | |
25 | index c2a87c8..1a96cac 100644 | |
26 | --- a/fs/ocfs2/file.c | |
27 | +++ b/fs/ocfs2/file.c | |
28 | @@ -894,9 +894,9 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) | |
29 | struct ocfs2_super *osb = OCFS2_SB(sb); | |
30 | struct buffer_head *bh = NULL; | |
31 | handle_t *handle = NULL; | |
32 | - int locked[MAXQUOTAS] = {0, 0}; | |
33 | - int credits, qtype; | |
34 | - struct ocfs2_mem_dqinfo *oinfo; | |
35 | + int qtype; | |
36 | + struct dquot *transfer_from[MAXQUOTAS] = { }; | |
37 | + struct dquot *transfer_to[MAXQUOTAS] = { }; | |
38 | ||
39 | mlog_entry("(0x%p, '%.*s')\n", dentry, | |
40 | dentry->d_name.len, dentry->d_name.name); | |
41 | @@ -969,30 +969,37 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) | |
42 | ||
43 | if ((attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) || | |
44 | (attr->ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) { | |
45 | - credits = OCFS2_INODE_UPDATE_CREDITS; | |
46 | + /* | |
47 | + * Gather pointers to quota structures so that allocation / | |
48 | + * freeing of quota structures happens here and not inside | |
49 | + * vfs_dq_transfer() where we have problems with lock ordering | |
50 | + */ | |
51 | if (attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid | |
52 | && OCFS2_HAS_RO_COMPAT_FEATURE(sb, | |
53 | OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) { | |
54 | - oinfo = sb_dqinfo(sb, USRQUOTA)->dqi_priv; | |
55 | - status = ocfs2_lock_global_qf(oinfo, 1); | |
56 | - if (status < 0) | |
57 | + transfer_to[USRQUOTA] = dqget(sb, attr->ia_uid, | |
58 | + USRQUOTA); | |
59 | + transfer_from[USRQUOTA] = dqget(sb, inode->i_uid, | |
60 | + USRQUOTA); | |
61 | + if (!transfer_to[USRQUOTA] || !transfer_from[USRQUOTA]) { | |
62 | + status = -ESRCH; | |
63 | goto bail_unlock; | |
64 | - credits += ocfs2_calc_qinit_credits(sb, USRQUOTA) + | |
65 | - ocfs2_calc_qdel_credits(sb, USRQUOTA); | |
66 | - locked[USRQUOTA] = 1; | |
67 | + } | |
68 | } | |
69 | if (attr->ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid | |
70 | && OCFS2_HAS_RO_COMPAT_FEATURE(sb, | |
71 | OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) { | |
72 | - oinfo = sb_dqinfo(sb, GRPQUOTA)->dqi_priv; | |
73 | - status = ocfs2_lock_global_qf(oinfo, 1); | |
74 | - if (status < 0) | |
75 | + transfer_to[GRPQUOTA] = dqget(sb, attr->ia_gid, | |
76 | + GRPQUOTA); | |
77 | + transfer_from[GRPQUOTA] = dqget(sb, inode->i_gid, | |
78 | + GRPQUOTA); | |
79 | + if (!transfer_to[GRPQUOTA] || !transfer_from[GRPQUOTA]) { | |
80 | + status = -ESRCH; | |
81 | goto bail_unlock; | |
82 | - credits += ocfs2_calc_qinit_credits(sb, GRPQUOTA) + | |
83 | - ocfs2_calc_qdel_credits(sb, GRPQUOTA); | |
84 | - locked[GRPQUOTA] = 1; | |
85 | + } | |
86 | } | |
87 | - handle = ocfs2_start_trans(osb, credits); | |
88 | + handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS + | |
89 | + 2 * ocfs2_quota_trans_credits(sb)); | |
90 | if (IS_ERR(handle)) { | |
91 | status = PTR_ERR(handle); | |
92 | mlog_errno(status); | |
93 | @@ -1030,12 +1037,6 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) | |
94 | bail_commit: | |
95 | ocfs2_commit_trans(osb, handle); | |
96 | bail_unlock: | |
97 | - for (qtype = 0; qtype < MAXQUOTAS; qtype++) { | |
98 | - if (!locked[qtype]) | |
99 | - continue; | |
100 | - oinfo = sb_dqinfo(sb, qtype)->dqi_priv; | |
101 | - ocfs2_unlock_global_qf(oinfo, 1); | |
102 | - } | |
103 | ocfs2_inode_unlock(inode, 1); | |
104 | bail_unlock_rw: | |
105 | if (size_change) | |
106 | @@ -1043,6 +1044,12 @@ bail_unlock_rw: | |
107 | bail: | |
108 | brelse(bh); | |
109 | ||
110 | + /* Release quota pointers in case we acquired them */ | |
111 | + for (qtype = 0; qtype < MAXQUOTAS; qtype++) { | |
112 | + dqput(transfer_to[qtype]); | |
113 | + dqput(transfer_from[qtype]); | |
114 | + } | |
115 | + | |
116 | if (!status && attr->ia_valid & ATTR_MODE) { | |
117 | status = ocfs2_acl_chmod(inode); | |
118 | if (status < 0) | |
119 | -- | |
120 | 1.6.0.2 | |
121 |