]>
Commit | Line | Data |
---|---|---|
2cb7cef9 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 file changed, 30 insertions(+), 23 deletions(-) | |
23 | ||
24 | --- a/fs/ocfs2/file.c | |
25 | +++ b/fs/ocfs2/file.c | |
26 | @@ -899,9 +899,9 @@ int ocfs2_setattr(struct dentry *dentry, | |
27 | struct ocfs2_super *osb = OCFS2_SB(sb); | |
28 | struct buffer_head *bh = NULL; | |
29 | handle_t *handle = NULL; | |
30 | - int locked[MAXQUOTAS] = {0, 0}; | |
31 | - int credits, qtype; | |
32 | - struct ocfs2_mem_dqinfo *oinfo; | |
33 | + int qtype; | |
34 | + struct dquot *transfer_from[MAXQUOTAS] = { }; | |
35 | + struct dquot *transfer_to[MAXQUOTAS] = { }; | |
36 | ||
37 | mlog_entry("(0x%p, '%.*s')\n", dentry, | |
38 | dentry->d_name.len, dentry->d_name.name); | |
39 | @@ -974,30 +974,37 @@ int ocfs2_setattr(struct dentry *dentry, | |
40 | ||
41 | if ((attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid) || | |
42 | (attr->ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid)) { | |
43 | - credits = OCFS2_INODE_UPDATE_CREDITS; | |
44 | + /* | |
45 | + * Gather pointers to quota structures so that allocation / | |
46 | + * freeing of quota structures happens here and not inside | |
47 | + * vfs_dq_transfer() where we have problems with lock ordering | |
48 | + */ | |
49 | if (attr->ia_valid & ATTR_UID && attr->ia_uid != inode->i_uid | |
50 | && OCFS2_HAS_RO_COMPAT_FEATURE(sb, | |
51 | OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) { | |
52 | - oinfo = sb_dqinfo(sb, USRQUOTA)->dqi_priv; | |
53 | - status = ocfs2_lock_global_qf(oinfo, 1); | |
54 | - if (status < 0) | |
55 | + transfer_to[USRQUOTA] = dqget(sb, attr->ia_uid, | |
56 | + USRQUOTA); | |
57 | + transfer_from[USRQUOTA] = dqget(sb, inode->i_uid, | |
58 | + USRQUOTA); | |
59 | + if (!transfer_to[USRQUOTA] || !transfer_from[USRQUOTA]) { | |
60 | + status = -ESRCH; | |
61 | goto bail_unlock; | |
62 | - credits += ocfs2_calc_qinit_credits(sb, USRQUOTA) + | |
63 | - ocfs2_calc_qdel_credits(sb, USRQUOTA); | |
64 | - locked[USRQUOTA] = 1; | |
65 | + } | |
66 | } | |
67 | if (attr->ia_valid & ATTR_GID && attr->ia_gid != inode->i_gid | |
68 | && OCFS2_HAS_RO_COMPAT_FEATURE(sb, | |
69 | OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) { | |
70 | - oinfo = sb_dqinfo(sb, GRPQUOTA)->dqi_priv; | |
71 | - status = ocfs2_lock_global_qf(oinfo, 1); | |
72 | - if (status < 0) | |
73 | + transfer_to[GRPQUOTA] = dqget(sb, attr->ia_gid, | |
74 | + GRPQUOTA); | |
75 | + transfer_from[GRPQUOTA] = dqget(sb, inode->i_gid, | |
76 | + GRPQUOTA); | |
77 | + if (!transfer_to[GRPQUOTA] || !transfer_from[GRPQUOTA]) { | |
78 | + status = -ESRCH; | |
79 | goto bail_unlock; | |
80 | - credits += ocfs2_calc_qinit_credits(sb, GRPQUOTA) + | |
81 | - ocfs2_calc_qdel_credits(sb, GRPQUOTA); | |
82 | - locked[GRPQUOTA] = 1; | |
83 | + } | |
84 | } | |
85 | - handle = ocfs2_start_trans(osb, credits); | |
86 | + handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS + | |
87 | + 2 * ocfs2_quota_trans_credits(sb)); | |
88 | if (IS_ERR(handle)) { | |
89 | status = PTR_ERR(handle); | |
90 | mlog_errno(status); | |
91 | @@ -1035,12 +1042,6 @@ int ocfs2_setattr(struct dentry *dentry, | |
92 | bail_commit: | |
93 | ocfs2_commit_trans(osb, handle); | |
94 | bail_unlock: | |
95 | - for (qtype = 0; qtype < MAXQUOTAS; qtype++) { | |
96 | - if (!locked[qtype]) | |
97 | - continue; | |
98 | - oinfo = sb_dqinfo(sb, qtype)->dqi_priv; | |
99 | - ocfs2_unlock_global_qf(oinfo, 1); | |
100 | - } | |
101 | ocfs2_inode_unlock(inode, 1); | |
102 | bail_unlock_rw: | |
103 | if (size_change) | |
104 | @@ -1048,6 +1049,12 @@ bail_unlock_rw: | |
105 | bail: | |
106 | brelse(bh); | |
107 | ||
108 | + /* Release quota pointers in case we acquired them */ | |
109 | + for (qtype = 0; qtype < MAXQUOTAS; qtype++) { | |
110 | + dqput(transfer_to[qtype]); | |
111 | + dqput(transfer_from[qtype]); | |
112 | + } | |
113 | + | |
114 | if (!status && attr->ia_valid & ATTR_MODE) { | |
115 | status = ocfs2_acl_chmod(inode); | |
116 | if (status < 0) |