- **status**: review --> fixed
- **Milestone**: 4.4.2 --> 4.5.1
- **Comment**:
changeset: 6027:8c10e46103c0
branch: opensaf-4.5.x
parent: 6025:3c62e3091aca
user: Anders Widell
date: Thu Oct 16 10:01:58 2014 +0200
summary: smf: Fix memory corruption and resource leak when CLM returns
error [#1166]
changeset: 6028:419f8f318539
tag: tip
parent: 6026:efb6fcef89ec
user: Anders Widell
date: Thu Oct 16 10:01:58 2014 +0200
summary: smf: Fix memory corruption and resource leak when CLM returns
error [#1166]
[staging:8c10e4]
[staging:419f8f]
---
** [tickets:#1166] smf: Memory corruption when handling node up event**
**Status:** fixed
**Milestone:** 4.5.1
**Created:** Thu Oct 09, 2014 11:59 AM UTC by Anders Widell
**Last Updated:** Fri Oct 10, 2014 02:46 PM UTC
**Owner:** nobody
In function smfnd_up() in file smfd_smfnd.c, there is a race between receiving
events from MDS and from CLM. When an MDS node up event has been received, the
CLM function saClmClusterNodeGet() is called to fetch information about the
node. If CLM does not yet know about the node, it may return an error code.
When this happens, three problems are present in the code below:
/* Check if the node id does already exists */
pthread_mutex_lock(&smfnd_list_lock);
smfnd = get_smfnd(i_node_id);
pthread_mutex_unlock(&smfnd_list_lock);
if (smfnd == NULL) {
TRACE("New node Id, create new SmfndNodeT structure");
smfnd = calloc(1, sizeof(SmfndNodeT));
if (smfnd == NULL) {
LOG_ER("alloc of SmfndNodeT failed");
return NCSCC_RC_FAILURE;
}
newNode = true;
}
/* Find Clm info about the node */
rc = saClmInitialize(&clmHandle, NULL, &clmVersion);
if (rc != SA_AIS_OK) {
LOG_ER("saClmInitialize failed, rc=%s", saf_error(rc));
return NCSCC_RC_FAILURE;
}
/* Get Clm info about the node */
rc = saClmClusterNodeGet(clmHandle, i_node_id,
10000000000LL, &smfnd->clmInfo);
if (rc != SA_AIS_OK) {
LOG_ER("saClmClusterNodeGet failed, rc=%s", saf_error(rc));
free(smfnd);
return NCSCC_RC_FAILURE;
}
rc = saClmFinalize(clmHandle);
First of all, free(smfnd) is called on a pointer that can either have been
returned from get_smfnd(), or from calloc(). We should only call free() when
the pointer was returned by calloc() - calling it when it was returned by
get_smfnd() will free memory that is stored in a linked list, causing memory
corruption!
Secondly, the output from saClmClusterNodeGet() is stored in &smfnd->clmInfo.
In the case where saClmClusterNodeGet() returns an error, it may be that
&smfnd->clmInfo is clobbered. It would be safer to use a temporary variable on
the stack as output parameter to saClmClusterNodeGet(), and then copy it into
the data structure only in case saClmClusterNodeGet() returns OK.
Thirdly, we return from this function without calling saClmFinalize(), causing
a leak of CLM handle.
---
Sent from sourceforge.net because [email protected] is
subscribed to https://sourceforge.net/p/opensaf/tickets/
To unsubscribe from further messages, a project admin can change settings at
https://sourceforge.net/p/opensaf/admin/tickets/options. Or, if this is a
mailing list, you can unsubscribe from the mailing list.------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
_______________________________________________
Opensaf-tickets mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-tickets
