diff mbox series

ceph: fix potential race condition of i_cap_delay_list access

Message ID 20250610190529.516586-1-slava@dubeyko.com
State New
Headers show
Series ceph: fix potential race condition of i_cap_delay_list access | expand

Commit Message

Viacheslav Dubeyko June 10, 2025, 7:05 p.m. UTC 
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

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@ibm.com>
---
 fs/ceph/caps.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)
diff mbox series

Patch

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);
 }