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, vdubeyko@redhat.com
Subject: [PATCH] ceph: cleanup in check_new_map() method
Date: Thu, 11 Sep 2025 11:56:08 -0700 [thread overview]
Message-ID: <20250911185607.323452-2-slava@dubeyko.com> (raw)
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
The Coverity Scan service has reported a potential issue
in check_new_map() method [1]. The check_new_map() executes
checking of newmap->m_info on NULL in the beginning of
the method. However, it operates by newmap->m_info later
in the method without any check on NULL. Analysis of the code
flow shows that ceph_mdsmap_decode() guarantees the allocation
of m_info array. And check_new_map() never will be called
with newmap->m_info not allocated.
This patch exchanges checking of newmap->m_info on BUG_ON()
pattern because the situation of having NULL in newmap->m_info
during check_new_map() is not expecting event. Also, this patch
reworks logic of __open_export_target_sessions(),
ceph_mdsmap_get_addr(), ceph_mdsmap_get_state(), and
ceph_mdsmap_is_laggy() by checking mdsmap->m_info on NULL value.
[1] https://scan5.scan.coverity.com/#/project-view/64304/10063?selectedIssue=1491799
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/debugfs.c | 11 +++++++++--
fs/ceph/mds_client.c | 14 +++++++++-----
fs/ceph/mdsmap.h | 6 +++++-
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index fdd404fc8112..f1c49232eee0 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -37,8 +37,15 @@ static int mdsmap_show(struct seq_file *s, void *p)
seq_printf(s, "session_timeout %d\n", mdsmap->m_session_timeout);
seq_printf(s, "session_autoclose %d\n", mdsmap->m_session_autoclose);
for (i = 0; i < mdsmap->possible_max_rank; i++) {
- struct ceph_entity_addr *addr = &mdsmap->m_info[i].addr;
- int state = mdsmap->m_info[i].state;
+ struct ceph_entity_addr *addr;
+ int state;
+
+ if (unlikely(!mdsmap->m_info))
+ break;
+
+ addr = &mdsmap->m_info[i].addr;
+ state = mdsmap->m_info[i].state;
+
seq_printf(s, "\tmds%d\t%s\t(%s)\n", i,
ceph_pr_addr(addr),
ceph_mds_state_name(state));
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 0f497c39ff82..501b78f9de56 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1737,6 +1737,9 @@ static void __open_export_target_sessions(struct ceph_mds_client *mdsc,
if (mds >= mdsc->mdsmap->possible_max_rank)
return;
+ if (unlikely(!mdsc->mdsmap->m_info))
+ return;
+
mi = &mdsc->mdsmap->m_info[mds];
doutc(cl, "for mds%d (%d targets)\n", session->s_mds,
mi->num_export_targets);
@@ -5015,11 +5018,12 @@ static void check_new_map(struct ceph_mds_client *mdsc,
doutc(cl, "new %u old %u\n", newmap->m_epoch, oldmap->m_epoch);
- if (newmap->m_info) {
- for (i = 0; i < newmap->possible_max_rank; i++) {
- for (j = 0; j < newmap->m_info[i].num_export_targets; j++)
- set_bit(newmap->m_info[i].export_targets[j], targets);
- }
+ /* ceph_mdsmap_decode() should guarantee m_info allocation */
+ BUG_ON(!newmap->m_info);
+
+ for (i = 0; i < newmap->possible_max_rank; i++) {
+ for (j = 0; j < newmap->m_info[i].num_export_targets; j++)
+ set_bit(newmap->m_info[i].export_targets[j], targets);
}
for (i = 0; i < oldmap->possible_max_rank && i < mdsc->max_sessions; i++) {
diff --git a/fs/ceph/mdsmap.h b/fs/ceph/mdsmap.h
index 1f2171dd01bf..12530f25a63b 100644
--- a/fs/ceph/mdsmap.h
+++ b/fs/ceph/mdsmap.h
@@ -52,6 +52,8 @@ ceph_mdsmap_get_addr(struct ceph_mdsmap *m, int w)
{
if (w >= m->possible_max_rank)
return NULL;
+ if (unlikely(!m->m_info))
+ return NULL;
return &m->m_info[w].addr;
}
@@ -60,12 +62,14 @@ static inline int ceph_mdsmap_get_state(struct ceph_mdsmap *m, int w)
BUG_ON(w < 0);
if (w >= m->possible_max_rank)
return CEPH_MDS_STATE_DNE;
+ if (unlikely(!m->m_info))
+ return CEPH_MDS_STATE_DNE;
return m->m_info[w].state;
}
static inline bool ceph_mdsmap_is_laggy(struct ceph_mdsmap *m, int w)
{
- if (w >= 0 && w < m->possible_max_rank)
+ if (w >= 0 && w < m->possible_max_rank && m->m_info)
return m->m_info[w].laggy;
return false;
}
--
2.51.0
reply other threads:[~2025-09-11 18:56 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=20250911185607.323452-2-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 \
--cc=vdubeyko@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