| Message ID | 9443fd34-fb39-2edb-0a65-8e782bd1e064@dev.mellanox.co.il (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [PATCHv2,opensm] osm_sa_mcmember_record.c: Fix use after free in mcmr_rcv_join_mgrp | expand |
On Wed, Aug 08, 2018 at 05:30:03AM -0400, Hal Rosenstock wrote: > > The cleanup function frees p_mgrp before the osm_log() prints the GUID. > > Issue was found by Coverity. > > Error: USE_AFTER_FREE (CWE-825): [#def11] > opensm-3.3.20/opensm/osm_sa_mcmember_record.c:1340: freed_arg: "osm_mgrp_cleanup" frees "p_mgrp". > opensm-3.3.20/opensm/osm_multicast.c:184:2: freed_arg: "free" frees parameter "mgrp". > opensm-3.3.20/opensm/osm_sa_mcmember_record.c:1342: deref_after_free: Dereferencing freed pointer "p_mgrp". > |# 1340| osm_mgrp_cleanup(sa->p_subn, p_mgrp); > |# 1341| CL_PLOCK_RELEASE(sa->p_lock); > |# 1342|-> OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 1B12: " > |# 1343| "validate_more_comp_fields, validate_port_caps, " > |# 1344| "or JoinState = 0 failed for MGID: %s port 0x%016" PRIx64 > > Error: USE_AFTER_FREE (CWE-825): [#def12] > opensm-3.3.20/opensm/osm_sa_mcmember_record.c:1357: freed_arg: "osm_mgrp_cleanup" frees "p_mgrp". > opensm-3.3.20/opensm/osm_multicast.c:184:2: freed_arg: "free" frees parameter "mgrp". > opensm-3.3.20/opensm/osm_sa_mcmember_record.c:1360: deref_after_free: Dereferencing freed pointer "p_mgrp". > |# 1358| CL_PLOCK_RELEASE(sa->p_lock); > |# 1359| memset(gid_str, 0, sizeof(gid_str)); > |# 1360|-> OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 1B14: " > |# 1361| "Cannot join port 0x%016" PRIx64 " to MGID %s - " > |# 1362| > > Signed-off-by: Dan Ben Yosef <danby@mellanox.com> > Signed-off-by: Alex Netes <alexne@mellanox.com> > Signed-off-by: Hal Rosenstock <hal@mellanox.com> > --- > opensm/osm_sa_mcmember_record.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/opensm/osm_sa_mcmember_record.c b/opensm/osm_sa_mcmember_record.c > index 9b94993..cf74308 100644 > --- a/opensm/osm_sa_mcmember_record.c > +++ b/opensm/osm_sa_mcmember_record.c > @@ -1345,6 +1345,12 @@ static void mcmr_rcv_join_mgrp(IN osm_sa_t * sa, IN osm_madw_t * p_madw) > || !validate_port_caps(sa->p_log, p_mgrp, p_physp) > || !(join_state != 0)) { > char gid_str[INET6_ADDRSTRLEN]; > + memset(gid_str, 0, sizeof(gid_str)); > + > + /* get the gid_str before the cleanup, the cleanup can free the pointer */ > + inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw, gid_str, > + sizeof gid_str); > + > /* since we might have created the new group we need to cleanup */ > if (is_new_group) > osm_mgrp_cleanup(sa->p_subn, p_mgrp); > @@ -1353,9 +1359,7 @@ static void mcmr_rcv_join_mgrp(IN osm_sa_t * sa, IN osm_madw_t * p_madw) > "validate_more_comp_fields, validate_port_caps, " > "or JoinState = 0 failed for MGID: %s port 0x%016" PRIx64 > " (%s), sending IB_SA_MAD_STATUS_REQ_INVALID\n", > - inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw, > - gid_str, sizeof gid_str), > - cl_ntoh64(portguid), p_port->p_node->print_desc); > + gid_str, cl_ntoh64(portguid), p_port->p_node->print_desc); > osm_sa_send_error(sa, p_madw, IB_SA_MAD_STATUS_REQ_INVALID); > goto Exit; > } > @@ -1363,17 +1367,17 @@ static void mcmr_rcv_join_mgrp(IN osm_sa_t * sa, IN osm_madw_t * p_madw) > /* verify that the joining port is in the partition of the group */ > if (!osm_physp_has_pkey(sa->p_log, p_mgrp->mcmember_rec.pkey, p_physp)) { > char gid_str[INET6_ADDRSTRLEN]; > + memset(gid_str, 0, sizeof(gid_str)); > + inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw, gid_str, > + sizeof(gid_str)); > + > if (is_new_group) > osm_mgrp_cleanup(sa->p_subn, p_mgrp); > CL_PLOCK_RELEASE(sa->p_lock); > - memset(gid_str, 0, sizeof(gid_str)); > OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 1B14: " > "Cannot join port 0x%016" PRIx64 " to MGID %s - " > "Port is not in partition of this MC group\n", > - cl_ntoh64(portguid), > - inet_ntop(AF_INET6, > - p_mgrp->mcmember_rec.mgid.raw, > - gid_str, sizeof(gid_str))); > + cl_ntoh64(portguid), gid_str); > osm_sa_send_error(sa, p_madw, IB_SA_MAD_STATUS_REQ_INVALID); > goto Exit; > } Acked-by: Honggang Li <honli@redhat.com> thanks > -- > 2.8.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/opensm/osm_sa_mcmember_record.c b/opensm/osm_sa_mcmember_record.c index 9b94993..cf74308 100644 --- a/opensm/osm_sa_mcmember_record.c +++ b/opensm/osm_sa_mcmember_record.c @@ -1345,6 +1345,12 @@ static void mcmr_rcv_join_mgrp(IN osm_sa_t * sa, IN osm_madw_t * p_madw) || !validate_port_caps(sa->p_log, p_mgrp, p_physp) || !(join_state != 0)) { char gid_str[INET6_ADDRSTRLEN]; + memset(gid_str, 0, sizeof(gid_str)); + + /* get the gid_str before the cleanup, the cleanup can free the pointer */ + inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw, gid_str, + sizeof gid_str); + /* since we might have created the new group we need to cleanup */ if (is_new_group) osm_mgrp_cleanup(sa->p_subn, p_mgrp); @@ -1353,9 +1359,7 @@ static void mcmr_rcv_join_mgrp(IN osm_sa_t * sa, IN osm_madw_t * p_madw) "validate_more_comp_fields, validate_port_caps, " "or JoinState = 0 failed for MGID: %s port 0x%016" PRIx64 " (%s), sending IB_SA_MAD_STATUS_REQ_INVALID\n", - inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw, - gid_str, sizeof gid_str), - cl_ntoh64(portguid), p_port->p_node->print_desc); + gid_str, cl_ntoh64(portguid), p_port->p_node->print_desc); osm_sa_send_error(sa, p_madw, IB_SA_MAD_STATUS_REQ_INVALID); goto Exit; } @@ -1363,17 +1367,17 @@ static void mcmr_rcv_join_mgrp(IN osm_sa_t * sa, IN osm_madw_t * p_madw) /* verify that the joining port is in the partition of the group */ if (!osm_physp_has_pkey(sa->p_log, p_mgrp->mcmember_rec.pkey, p_physp)) { char gid_str[INET6_ADDRSTRLEN]; + memset(gid_str, 0, sizeof(gid_str)); + inet_ntop(AF_INET6, p_mgrp->mcmember_rec.mgid.raw, gid_str, + sizeof(gid_str)); + if (is_new_group) osm_mgrp_cleanup(sa->p_subn, p_mgrp); CL_PLOCK_RELEASE(sa->p_lock); - memset(gid_str, 0, sizeof(gid_str)); OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 1B14: " "Cannot join port 0x%016" PRIx64 " to MGID %s - " "Port is not in partition of this MC group\n", - cl_ntoh64(portguid), - inet_ntop(AF_INET6, - p_mgrp->mcmember_rec.mgid.raw, - gid_str, sizeof(gid_str))); + cl_ntoh64(portguid), gid_str); osm_sa_send_error(sa, p_madw, IB_SA_MAD_STATUS_REQ_INVALID); goto Exit; }