From: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx> The Coverity Scan service has detected potential race condition of i_cap_delay_list access [1]. The CID 1596363 contains explanation: "Accessing ci->i_cap_delay_list without holding lock ceph_mds_client.cap_delay_lock. Elsewhere, ceph_inode_info.i_cap_delay_list is written to with ceph_mds_client.cap_delay_lock held 9 out of 9 times. The value of the shared data will be determined by the interleaving of thread execution. In ceph_check_caps: Thread shared data is accessed without holding an appropriate lock, possibly causing a race condition (CWE-366)". The patch reworks __cap_delay_cancel() logic by means moving list_empty(&ci->i_cap_delay_list) under mdsc->cap_delay_lock protection. Patch introduces is_cap_delay_list_empty_safe() function that checks the emptiness of i_cap_delay_list under mdsc->cap_delay_lock protection. This function is used in ceph_check_caps() and __ceph_touch_fmode() methods to resolve the race condition issue. [1] https://scan5.scan.coverity.com/#/project-view/64304/10063?selectedIssue=1596363 Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx> --- fs/ceph/caps.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c index a8d8b56cf9d2..eceee464ec50 100644 --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -566,13 +566,26 @@ static void __cap_delay_cancel(struct ceph_mds_client *mdsc, struct inode *inode = &ci->netfs.inode; doutc(mdsc->fsc->client, "%p %llx.%llx\n", inode, ceph_vinop(inode)); - if (list_empty(&ci->i_cap_delay_list)) - return; + spin_lock(&mdsc->cap_delay_lock); - list_del_init(&ci->i_cap_delay_list); + if (!list_empty(&ci->i_cap_delay_list)) { + list_del_init(&ci->i_cap_delay_list); + } spin_unlock(&mdsc->cap_delay_lock); } +static inline bool is_cap_delay_list_empty_safe(struct ceph_mds_client *mdsc, + struct ceph_inode_info *ci) +{ + bool is_empty; + + spin_lock(&mdsc->cap_delay_lock); + is_empty = list_empty(&ci->i_cap_delay_list); + spin_unlock(&mdsc->cap_delay_lock); + + return is_empty; +} + /* Common issue checks for add_cap, handle_cap_grant. */ static void __check_cap_issue(struct ceph_inode_info *ci, struct ceph_cap *cap, unsigned issued) @@ -2260,7 +2273,7 @@ void ceph_check_caps(struct ceph_inode_info *ci, int flags) /* periodically re-calculate caps wanted by open files */ if (__ceph_is_any_real_caps(ci) && - list_empty(&ci->i_cap_delay_list) && + is_cap_delay_list_empty_safe(mdsc, ci) && (file_wanted & ~CEPH_CAP_PIN) && !(used & (CEPH_CAP_FILE_RD | CEPH_CAP_ANY_FILE_WR))) { __cap_delay_requeue(mdsc, ci); @@ -4720,7 +4733,7 @@ void __ceph_touch_fmode(struct ceph_inode_info *ci, /* queue periodic check */ if (fmode && __ceph_is_any_real_caps(ci) && - list_empty(&ci->i_cap_delay_list)) + is_cap_delay_list_empty_safe(mdsc, ci)) __cap_delay_requeue(mdsc, ci); } -- 2.49.0
