On 8/7/2018 7:33 PM, Honggang LI wrote:
> On Tue, Aug 07, 2018 at 09:44: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
>>
>
> This patch fixes #def11, but does not fix #def12
>
> NAK and REJECT.
Sorry. I missed update of second case. Updated patch to follow shortly.
>> 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|
>
> This issue persists.
>
>>
>> Signed-off-by: Dan Ben Yosef <danby@xxxxxxxxxxxx>
>> Signed-off-by: Alex Netes <alexne@xxxxxxxxxxxx>
>> Signed-off-by: Hal Rosenstock <hal@xxxxxxxxxxxx>
>> ---
>> opensm/osm_sa_mcmember_record.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/opensm/osm_sa_mcmember_record.c b/opensm/osm_sa_mcmember_record.c
>> index 9b94993..e113416 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;
>> }
>> --
>> 2.8.4
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
