Skip to content

Commit 5824ccb

Browse files
dubeykoidryomov
authored andcommitted
ceph: fix potential race condition in ceph_ioctl_lazyio()
The Coverity Scan service has detected potential race condition in ceph_ioctl_lazyio() [1]. The CID 1591046 contains explanation: "Check of thread-shared field evades lock acquisition (LOCK_EVASION). Thread1 sets fmode to a new value. Now the two threads have an inconsistent view of fmode and updates to fields correlated with fmode may be lost. The data guarded by this critical section may be read while in an inconsistent state or modified by multiple racing threads. In ceph_ioctl_lazyio: Checking the value of a thread-shared field outside of a locked region to determine if a locked operation involving that thread shared field has completed. (CWE-543)". The patch places fi->fmode field access under ci->i_ceph_lock protection. Also, it introduces the is_file_already_lazy variable that is set under the lock and it is checked later out of scope of critical section. [1] https://scan5.scan.coverity.com/#/project-view/64304/10063?selectedIssue=1591046 Signed-off-by: Viacheslav Dubeyko <[email protected]> Reviewed-by: Alex Markuze <[email protected]> Signed-off-by: Ilya Dryomov <[email protected]>
1 parent 5b2d137 commit 5824ccb

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

fs/ceph/ioctl.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,21 +246,28 @@ static long ceph_ioctl_lazyio(struct file *file)
246246
struct ceph_inode_info *ci = ceph_inode(inode);
247247
struct ceph_mds_client *mdsc = ceph_inode_to_fs_client(inode)->mdsc;
248248
struct ceph_client *cl = mdsc->fsc->client;
249+
bool is_file_already_lazy = false;
249250

251+
spin_lock(&ci->i_ceph_lock);
250252
if ((fi->fmode & CEPH_FILE_MODE_LAZY) == 0) {
251-
spin_lock(&ci->i_ceph_lock);
252253
fi->fmode |= CEPH_FILE_MODE_LAZY;
253254
ci->i_nr_by_mode[ffs(CEPH_FILE_MODE_LAZY)]++;
254255
__ceph_touch_fmode(ci, mdsc, fi->fmode);
255-
spin_unlock(&ci->i_ceph_lock);
256+
} else {
257+
is_file_already_lazy = true;
258+
}
259+
spin_unlock(&ci->i_ceph_lock);
260+
261+
if (is_file_already_lazy) {
262+
doutc(cl, "file %p %p %llx.%llx already lazy\n", file, inode,
263+
ceph_vinop(inode));
264+
} else {
256265
doutc(cl, "file %p %p %llx.%llx marked lazy\n", file, inode,
257266
ceph_vinop(inode));
258267

259268
ceph_check_caps(ci, 0);
260-
} else {
261-
doutc(cl, "file %p %p %llx.%llx already lazy\n", file, inode,
262-
ceph_vinop(inode));
263269
}
270+
264271
return 0;
265272
}
266273

0 commit comments

Comments
 (0)