ceph: cleanup in check_new_map() method

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>

diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 7dc3077902401ed00c7f2e353d7f948964f7d65d..e54020eded1be2ca902801df15121ca465fbff65 100644 (file)
--- 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 cc3d8d86ef256104663645dc4ccb8d32c94abb56..677d4af572a936ecdb982a994c93eaa149872f43 100644 (file)
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1739,6 +1739,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);
@@ -5111,11 +5114,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 d48d07c3516d447a8f3e684f6ded1f1ca1674417..c3d8c0ad6aba6d77f9811f2dbed5730361641964 100644 (file)
--- a/fs/ceph/mdsmap.h
+++ b/fs/ceph/mdsmap.h
@@ -53,6 +53,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;
 }
 
@@ -61,12 +63,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;
 }