From: Viacheslav Dubeyko <slava@dubeyko.com>
To: ceph-devel@vger.kernel.org
Cc: idryomov@gmail.com, linux-fsdevel@vger.kernel.org,
pdonnell@redhat.com, amarkuze@redhat.com, Slava.Dubeyko@ibm.com,
slava@dubeyko.com
Subject: [PATCH] ceph: cleanup in __ceph_do_pending_vmtruncate() method
Date: Wed, 30 Jul 2025 11:54:11 -0700 [thread overview]
Message-ID: <20250730185411.1105738-1-slava@dubeyko.com> (raw)
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
The Coverity Scan service has detected an unchecked return
value in __ceph_do_pending_vmtruncate() method [1].
The CID 114041 contains explanation: " Calling
filemap_write_and_wait_range without checking return value.
If the function returns an error value, the error value
may be mistaken for a normal value. Value returned from
a function is not checked for errors before being used.
(CWE-252)".
The patch adds the checking of returned value of
filemap_write_and_wait_range() and reporting the error
message if something wrong is happened during the call.
It reworks the logic by removing the jump to retry
label because it could be the reason of potential
infinite loop in the case of error condition during
the filemap_write_and_wait_range() call. It was removed
the check to == ci->i_truncate_pagecache_size because
the to variable is set by ci->i_truncate_pagecache_size
in the above code logic. The uneccessary finish variable
has been removed because the to variable always has
ci->i_truncate_pagecache_size value. Now if the condition
ci->i_truncate_pending == 0 is true then logic will jump
to the end of the function and wake_up_all(&ci->i_cap_wq)
will be called.
[1] https://scan5.scan.coverity.com/#/project-view/64304/10063?selectedIssue=114041
Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
cc: Alex Markuze <amarkuze@redhat.com>
cc: Ilya Dryomov <idryomov@gmail.com>
cc: Ceph Development <ceph-devel@vger.kernel.org>
---
fs/ceph/inode.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index fc543075b827..53ce776b04b5 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -2203,17 +2203,17 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
struct ceph_client *cl = ceph_inode_to_client(inode);
struct ceph_inode_info *ci = ceph_inode(inode);
u64 to;
- int wrbuffer_refs, finish = 0;
+ int wrbuffer_refs;
+ int err;
mutex_lock(&ci->i_truncate_mutex);
-retry:
spin_lock(&ci->i_ceph_lock);
+
+ wrbuffer_refs = ci->i_wrbuffer_ref;
if (ci->i_truncate_pending == 0) {
doutc(cl, "%p %llx.%llx none pending\n", inode,
ceph_vinop(inode));
- spin_unlock(&ci->i_ceph_lock);
- mutex_unlock(&ci->i_truncate_mutex);
- return;
+ goto out_unlock;
}
/*
@@ -2224,9 +2224,14 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
spin_unlock(&ci->i_ceph_lock);
doutc(cl, "%p %llx.%llx flushing snaps first\n", inode,
ceph_vinop(inode));
- filemap_write_and_wait_range(&inode->i_data, 0,
- inode->i_sb->s_maxbytes);
- goto retry;
+ err = filemap_write_and_wait_range(&inode->i_data, 0,
+ inode->i_sb->s_maxbytes);
+ spin_lock(&ci->i_ceph_lock);
+
+ if (unlikely(err)) {
+ pr_err_client(cl, "failed of flushing snaps: err %d\n",
+ err);
+ }
}
/* there should be no reader or writer */
@@ -2242,20 +2247,17 @@ void __ceph_do_pending_vmtruncate(struct inode *inode)
truncate_pagecache(inode, to);
spin_lock(&ci->i_ceph_lock);
- if (to == ci->i_truncate_pagecache_size) {
- ci->i_truncate_pending = 0;
- finish = 1;
- }
- spin_unlock(&ci->i_ceph_lock);
- if (!finish)
- goto retry;
+ ci->i_truncate_pending = 0;
+out_unlock:
+ spin_unlock(&ci->i_ceph_lock);
mutex_unlock(&ci->i_truncate_mutex);
if (wrbuffer_refs == 0)
ceph_check_caps(ci, 0);
wake_up_all(&ci->i_cap_wq);
+ return;
}
static void ceph_inode_work(struct work_struct *work)
--
2.50.1
reply other threads:[~2025-07-30 18:54 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250730185411.1105738-1-slava@dubeyko.com \
--to=slava@dubeyko.com \
--cc=Slava.Dubeyko@ibm.com \
--cc=amarkuze@redhat.com \
--cc=ceph-devel@vger.kernel.org \
--cc=idryomov@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=pdonnell@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).