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