From: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx> 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@xxxxxxx> cc: Alex Markuze <amarkuze@xxxxxxxxxx> cc: Ilya Dryomov <idryomov@xxxxxxxxx> cc: Ceph Development <ceph-devel@xxxxxxxxxxxxxxx> --- 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
