Hi Minh, Ack. But I have a remark that I think is important but it can be fixed later if there is not time enough now.
In general it is important to comment code in a way that makes it easier (faster) to understand. A documentary comment describing what a little bit complicated code block (e.g if complex structs, external functions, variables declared outside of function (global) or complicated tests/algorithms etc. are used) makes reviews, later maintenance etc. much easier and faster. A minimum documentation requirement should be to always give a function/method a header describing: - what it's doing and if needed how it's doing it e.g. if any special algorithm is used - important dependencies of things outside of the function e.g. usage of or even more important changing of global data - All the parameters including info about if they are used as input or given values as output - Return value and what it means Example: /** * Brief description * More description * * int a [in] * int *b [out] * return -1 on error */ My remark: -------------- Several new functions are added. Descriptions are missing All functions should have a header describing what it is doing, in and out parameters and return value. This shall be done for both global and static functions. Unfortunately the already existing code is not very well commented/documented but I think it's a good idea not to add more:-) Thanks Lennart > -----Original Message----- > From: Minh Hon Chau [mailto:[email protected]] > Sent: den 1 oktober 2014 05:43 > To: Anders Widell; Lennart Lund; [email protected]; > [email protected] > Cc: [email protected] > Subject: [PATCH 3 of 3] NTF: Add protection against longDn notification for > unadapted producer/consumer [#1114] V2 > > osaf/libs/agents/saf/ntfa/ntfa_api.c | 532 +++++++++++++++++++++++--- > ------- > osaf/libs/agents/saf/ntfa/ntfa_util.c | 39 ++- > osaf/libs/common/ntfsv/ntfsv_mem.c | 1 + > 3 files changed, 412 insertions(+), 160 deletions(-) > > > Currently if any unadapted longDn consumer receives notification having > extended SaNameT > in notification (notificationObject(s), notifyingObject(s), value type as > LDAP_NAME) > will be crashed > > The crash is due to invalid access to extended SaNameT > > The patch makes longDn notification invisible to unadapted consumer. Also if > unadapted > producer is going to send a longDn notification or unadapted consumer > specifies longDn > object for filter, the error code SA_AIS_ERR_INVALID_PARAM is returned. > Affected APIs: saNtfNotificationSend, saNtfNotificationReadNext, > saNtfNotificationReadInitialize, saNtfNotificationSubscribe, saNtfPtrValGet, > and > notificationCallback. > > diff --git a/osaf/libs/agents/saf/ntfa/ntfa_api.c > b/osaf/libs/agents/saf/ntfa/ntfa_api.c > --- a/osaf/libs/agents/saf/ntfa/ntfa_api.c > +++ b/osaf/libs/agents/saf/ntfa/ntfa_api.c > @@ -39,6 +39,194 @@ ntfa_cb_t ntfa_cb = { > /* list of subscriptions for this process */ > ntfa_subscriber_list_t *subscriberNoList = NULL; > > +static SaAisErrorT checkNtfValueTypeRange(SaNtfValueTypeT type) > +{ > + return (type < SA_NTF_VALUE_UINT8 || type > > SA_NTF_VALUE_ARRAY)? SA_AIS_ERR_INVALID_PARAM > + > : SA_AIS_OK; > +} > + > +static SaAisErrorT checkNtfValue(v_data* pvdata, SaNtfValueTypeT type, > SaNtfValueT* value) > +{ > + SaAisErrorT rc = SA_AIS_OK; > + SaNameT *ptr; > + if ((rc = checkNtfValueTypeRange(type)) != SA_AIS_OK) { > + TRACE_1("Invalid value type(%d)", type); > + return rc; > + } > + > + if (type == SA_NTF_VALUE_LDAP_NAME) { > + /* SaNameT must have length > 2 */ > + if (value->ptrVal.dataSize <= 2) > + return SA_AIS_ERR_INVALID_PARAM; > + > + /* Check SaNameT */ > + ptr = pvdata->p_base + value->ptrVal.dataOffset; > + if (!ntfsv_sanamet_is_valid(ptr)) > + return SA_AIS_ERR_INVALID_PARAM; > + > + /* Refill the extended SaNameT into variable data */ > + if (osaf_is_an_extended_name(ptr)) { > + size_t length_to_copy; > + void *p; > + if (value->ptrVal.dataSize < > (osaf_extended_name_length(ptr) + 2)) { > + LOG_ER("The allocated memory is not large > enough," > + " the object will be truncated > (%s)" > + , > osaf_extended_name_borrow(ptr)); > + length_to_copy = value->ptrVal.dataSize - 2; > > + } else > + length_to_copy = > osaf_extended_name_length(ptr); > + > + p = pvdata->p_base + value->ptrVal.dataOffset + 2; > + memcpy(p, osaf_extended_name_borrow(ptr), > length_to_copy); > + } else { > + if (value->ptrVal.dataSize > > SA_MAX_UNEXTENDED_NAME_LENGTH + 2) > + value->ptrVal.dataSize = > SA_MAX_UNEXTENDED_NAME_LENGTH + 2; > + } > + } > + > + return rc; > +} > + > +static SaAisErrorT getTypeFromValue(ntfa_notification_hdl_rec_t *not_hdl, > const SaNtfValueT *value > + , > SaNtfValueTypeT* type) > +{ > + SaAisErrorT rc = SA_AIS_ERR_NOT_EXIST; > + SaNtfNotificationHeaderT *not_header = NULL; > + SaNtfAlarmNotificationT* not_alarm; > + SaNtfSecurityAlarmNotificationT* not_sec_alarm; > + SaNtfAttributeChangeNotificationT* not_att_change; > + SaNtfObjectCreateDeleteNotificationT* not_obj; > + int i; > + > + if (not_hdl == NULL || value == NULL || type == NULL) > + return SA_AIS_ERR_INVALID_PARAM; > + > + switch (not_hdl->ntfNotificationType) { > + case SA_NTF_TYPE_ALARM: > + not_alarm = ¬_hdl- > >ntfNotification.ntfAlarmNotification; > + not_header = ¬_alarm->notificationHeader; > + for (i=0 ; i<not_alarm->numSpecificProblems ; i++) > + if (value == ¬_alarm- > >specificProblems[i].problemValue) { > + *type = not_alarm- > >specificProblems[i].problemType; > + rc = SA_AIS_OK; > + goto done; > + } > + > + for (i=0 ; i<not_alarm->numMonitoredAttributes ; > i++) > + if (value == ¬_alarm- > >monitoredAttributes[i].attributeValue) { > + *type = not_alarm- > >monitoredAttributes[i].attributeType; > + rc = SA_AIS_OK; > + goto done; > + } > + > + for (i=0 ; i<not_alarm->numProposedRepairActions ; > i++) > + if (value == ¬_alarm- > >proposedRepairActions[i].actionValue) { > + *type = not_alarm- > >proposedRepairActions[i].actionValueType; > + rc = SA_AIS_OK; > + goto done; > + } > + > + if (not_alarm->thresholdInformation != NULL && > + (value == ¬_alarm- > >thresholdInformation->thresholdValue || > + value == ¬_alarm- > >thresholdInformation->thresholdHysteresis || > + value == ¬_alarm- > >thresholdInformation->observedValue)) { > + *type = not_alarm- > >thresholdInformation->thresholdValueType; > + rc = SA_AIS_OK; > + goto done; > + } > + break; > + case SA_NTF_TYPE_SECURITY_ALARM: > + not_sec_alarm = ¬_hdl- > >ntfNotification.ntfSecurityAlarmNotification; > + not_header = ¬_sec_alarm->notificationHeader; > + if (not_sec_alarm->securityAlarmDetector != NULL > && > + value == ¬_sec_alarm- > >securityAlarmDetector->value) { > + *type = not_sec_alarm- > >securityAlarmDetector->valueType; > + rc = SA_AIS_OK; > + goto done; > + } > + > + if (not_sec_alarm->serviceUser != NULL && > + value == ¬_sec_alarm->serviceUser- > >value) { > + *type = not_sec_alarm- > >serviceUser->valueType; > + rc = SA_AIS_OK; > + goto done; > + } > + > + if (not_sec_alarm->serviceProvider != NULL && > + value == ¬_sec_alarm->serviceProvider- > >value) { > + *type = not_sec_alarm- > >serviceProvider->valueType; > + rc = SA_AIS_OK; > + goto done; > + } > + break; > + case SA_NTF_TYPE_STATE_CHANGE: > + not_header = ¬_hdl- > >ntfNotification.ntfStateChangeNotification.notificationHeader; > + break; > + > + case SA_NTF_TYPE_ATTRIBUTE_CHANGE: > + not_att_change = ¬_hdl- > >ntfNotification.ntfAttributeChangeNotification; > + not_header = ¬_att_change->notificationHeader; > + for (i=0 ; i<not_att_change->numAttributes ; i++) { > + if (value == ¬_att_change- > >changedAttributes[i].oldAttributeValue || > + value == ¬_att_change- > >changedAttributes[i].newAttributeValue) { > + *type = not_att_change- > >changedAttributes[i].attributeType; > + /* NTFIMCN has being used > SA_NTF_VALUE_LDAP_NAME as SA_NTF_VALUE_STRING > + * for changeAttributes, so it must > return SA_NTF_VALUE_STRING due to > + * backward compatibility > + */ > + if (*type == > SA_NTF_VALUE_LDAP_NAME) > + *type = > SA_NTF_VALUE_STRING; > + rc = SA_AIS_OK; > + goto done; > + } > + } > + break; > + > + case SA_NTF_TYPE_OBJECT_CREATE_DELETE: > + not_obj = ¬_hdl- > >ntfNotification.ntfObjectCreateDeleteNotification; > + not_header = ¬_obj->notificationHeader; > + for (i=0 ; i<not_obj->numAttributes ; i++) { > + if (value == ¬_obj- > >objectAttributes[i].attributeValue) { > + *type = not_obj- > >objectAttributes[i].attributeType; > + /* NTFIMCN has being used > SA_NTF_VALUE_LDAP_NAME as SA_NTF_VALUE_STRING > + * for objectAttributes, so it must > return SA_NTF_VALUE_STRING due to > + * backward compatibility > + */ > + if (*type == > SA_NTF_VALUE_LDAP_NAME) > + *type = > SA_NTF_VALUE_STRING; > + rc = SA_AIS_OK; > + goto done; > + } > + } > + break; > + default: > + TRACE_1("Unknown notification type"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + for (i=0 ; i<not_header->numAdditionalInfo ; i++) > + if (value == ¬_header->additionalInfo[i].infoValue) { > + *type = not_header->additionalInfo[i].infoType; > + rc = SA_AIS_OK; > + goto done; > + } > +done: > + return rc; > +} > + > +static SaAisErrorT checkFilterHeader(SaNtfNotificationFilterHeaderT *nfh) { > + > + SaUint16T i; > + for (i = 0; i != nfh->numNotificationObjects; ++i) > + if (!ntfsv_sanamet_is_valid(&nfh->notificationObjects[i])) > + return SA_AIS_ERR_INVALID_PARAM; > + for (i = 0; i != nfh->numNotifyingObjects; ++i) > + if (!ntfsv_sanamet_is_valid(&nfh->notifyingObjects[i])) > + return SA_AIS_ERR_INVALID_PARAM; > + > + return SA_AIS_OK; > +} > + > static SaAisErrorT > checkAttributeChangeFilterParameters(ntfa_filter_hdl_rec_t > *attributeChangeFilterData) > { > SaUint16T i; > @@ -72,8 +260,8 @@ static SaAisErrorT checkAttributeChangeF > } > } > > - TRACE_1("Returning SA_AIS_OK!"); > - return SA_AIS_OK; > + return checkFilterHeader(&attributeChangeFilterData- > >notificationFilter. > + > attributeChangeNotificationfilter.notificationFilterHeader); > > } > > @@ -111,8 +299,8 @@ static SaAisErrorT checkObjectCreateDele > } > } > > - TRACE_1("Returning SA_AIS_OK!"); > - return SA_AIS_OK; > + return checkFilterHeader(&objectCreateDeleteFilterData- > >notificationFilter. > + > objectCreateDeleteNotificationfilter.notificationFilterHeader); > > } > > @@ -155,9 +343,8 @@ static SaAisErrorT checkStateChangeFilte > return SA_AIS_ERR_INVALID_PARAM; > } > } > - > - TRACE_1("Returning SA_AIS_OK!"); > - return SA_AIS_OK; > + return checkFilterHeader(&stateChangeFilterData->notificationFilter. > + > stateChangeNotificationfilter.notificationFilterHeader); > } > > static SaAisErrorT checkAlarmFilterParameters(ntfa_filter_hdl_rec_t > *alarmFilterData) > @@ -209,8 +396,8 @@ static SaAisErrorT checkAlarmFilterParam > } > } > > - TRACE_1("Returning SA_AIS_OK!"); > - return SA_AIS_OK; > + return checkFilterHeader(&alarmFilterData->notificationFilter. > + > alarmNotificationfilter.notificationFilterHeader); > } > > static SaAisErrorT checkSecurityAlarmFilterParameters(ntfa_filter_hdl_rec_t > *securityAlarmFilterData) > @@ -260,11 +447,9 @@ static SaAisErrorT checkSecurityAlarmFil > for (i = 0; > i < securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter.numSecurityAlarmDetector > s; > i++) { > - if (securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > - securityAlarmDetectors[i].valueType < > SA_NTF_VALUE_UINT8 > - || securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > - securityAlarmDetectors[i].valueType > > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid PercievedSeverity value = %d", > + if (checkNtfValueTypeRange(securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > + > securityAlarmDetectors[i].valueType) != SA_AIS_OK) { > + TRACE_1("Invalid securityAlarmDetectors value type > = %d", > (int)securityAlarmFilterData- > >notificationFilter. > > securityAlarmNotificationfilter.securityAlarmDetectors[i].valueType); > return SA_AIS_ERR_INVALID_PARAM; > @@ -273,11 +458,9 @@ static SaAisErrorT checkSecurityAlarmFil > > for (i = 0; i < securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter.numServiceUsers; > i++) { > - if (securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > - serviceUsers[i].valueType < SA_NTF_VALUE_UINT8 > - || securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > - serviceUsers[i].valueType > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid PercievedSeverity value = %d", > + if (checkNtfValueTypeRange(securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > + > serviceUsers[i].valueType) != SA_AIS_OK) { > + TRACE_1("Invalid serviceUsers value type = %d", > (int)securityAlarmFilterData- > >notificationFilter. > > securityAlarmNotificationfilter.serviceUsers[i].valueType); > return SA_AIS_ERR_INVALID_PARAM; > @@ -286,26 +469,24 @@ static SaAisErrorT checkSecurityAlarmFil > > for (i = 0; i < securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter.numServiceProviders; > i++) { > - if (securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > - serviceProviders[i].valueType < SA_NTF_VALUE_UINT8 > - || securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > - serviceProviders[i].valueType > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid PercievedSeverity value = %d", > + if (checkNtfValueTypeRange(securityAlarmFilterData- > >notificationFilter.securityAlarmNotificationfilter. > + > serviceProviders[i].valueType) != SA_AIS_OK) { > + TRACE_1("Invalid serviceProviders value type = %d", > (int)securityAlarmFilterData- > >notificationFilter. > > securityAlarmNotificationfilter.serviceProviders[i].valueType); > return SA_AIS_ERR_INVALID_PARAM; > } > } > > - TRACE_1("Returning SA_AIS_OK!"); > - return SA_AIS_OK; > + return checkFilterHeader(&securityAlarmFilterData- > >notificationFilter. > + > securityAlarmNotificationfilter.notificationFilterHeader); > } > > /* help functions */ > -static SaAisErrorT checkHeader(SaNtfNotificationHeaderT *nh) > +static SaAisErrorT checkHeader(v_data *pvdata, SaNtfNotificationHeaderT > *nh) > { > int i =0; > - > + SaAisErrorT rc; > if (!ntfsv_sanamet_is_valid(nh->notificationObject) || > !ntfsv_sanamet_is_valid(nh->notifyingObject)) { > TRACE_1("SaNameT is invaild"); > @@ -313,80 +494,81 @@ static SaAisErrorT checkHeader(SaNtfNoti > } > > for(i=0 ; i < nh->numAdditionalInfo ; i++ ) { > - if(nh->additionalInfo[i].infoType < SA_NTF_VALUE_UINT8 || > - nh->additionalInfo[i].infoType > > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid numAdditionalInfo type value"); > + if ((rc = checkNtfValue(pvdata, > + nh- > >additionalInfo[i].infoType, > + &nh- > >additionalInfo[i].infoValue)) != SA_AIS_OK) > + return rc; > + } > + > + return SA_AIS_OK; > +} > + > +static SaAisErrorT checkAlarmParameters(ntfa_notification_hdl_rec_t > *not_hdl_rec, v_data *pvdata) > +{ > + int i = 0; > + SaNtfAlarmNotificationT *notification = ¬_hdl_rec- > >ntfNotification.ntfAlarmNotification; > + if (*notification->probableCause < SA_NTF_ADAPTER_ERROR || > + *notification->probableCause > SA_NTF_UNSPECIFIED_REASON) { > + TRACE_1("Invalid probableCause value"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + if (*notification->perceivedSeverity < SA_NTF_SEVERITY_CLEARED || > + *notification->perceivedSeverity > SA_NTF_SEVERITY_CRITICAL) { > + TRACE_1("Invalid perceivedSeverity value"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + if (*notification->trend < SA_NTF_TREND_MORE_SEVERE || > + *notification->trend > SA_NTF_TREND_LESS_SEVERE) { > + TRACE_1("Invalid trend value"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + for (i = 0; i < notification->numSpecificProblems; i++) { > + if (checkNtfValue(pvdata > + , notification- > >specificProblems[i].problemType > + , ¬ification- > >specificProblems[i].problemValue) != SA_AIS_OK) { > + TRACE_1("Invalid specificProblems"); > return SA_AIS_ERR_INVALID_PARAM; > } > } > > - return SA_AIS_OK; > -} > - > -static SaAisErrorT checkAlarmParameters(SaNtfAlarmNotificationT > *alarmNotification) > -{ > - int i = 0; > - > - if (*alarmNotification->probableCause < SA_NTF_ADAPTER_ERROR > || > - *alarmNotification->probableCause > > SA_NTF_UNSPECIFIED_REASON) { > - TRACE_1("Invalid probableCause value"); > - return SA_AIS_ERR_INVALID_PARAM; > + for (i = 0; i < notification->numMonitoredAttributes; i++) { > + if (checkNtfValue(pvdata > + , notification- > >monitoredAttributes[i].attributeType > + , ¬ification- > >monitoredAttributes[i].attributeValue) != SA_AIS_OK) { > + TRACE_1("Invalid monitoredAttributes"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > } > > - if (*alarmNotification->perceivedSeverity < > SA_NTF_SEVERITY_CLEARED || > - *alarmNotification->perceivedSeverity > > SA_NTF_SEVERITY_CRITICAL) { > - TRACE_1("Invalid perceivedSeverity value"); > - return SA_AIS_ERR_INVALID_PARAM; > + for (i = 0; i < notification->numProposedRepairActions; i++) { > + if (checkNtfValue(pvdata > + , notification- > >proposedRepairActions[i].actionValueType > + , ¬ification- > >proposedRepairActions[i].actionValue) != SA_AIS_OK) { > + TRACE_1("Invalid proposedRepairActions"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > } > > - if (*alarmNotification->trend < SA_NTF_TREND_MORE_SEVERE || > - *alarmNotification->trend > SA_NTF_TREND_LESS_SEVERE) { > - TRACE_1("Invalid trend value"); > - return SA_AIS_ERR_INVALID_PARAM; > - } > - > - for (i = 0; i < alarmNotification->numSpecificProblems; i++) { > - if (alarmNotification->specificProblems[i].problemType < > SA_NTF_VALUE_UINT8 || > - alarmNotification->specificProblems[i].problemType > > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid specific problem type value"); > - return SA_AIS_ERR_INVALID_PARAM; > - } > - } > - > - for (i = 0; i < alarmNotification->numMonitoredAttributes; i++) { > - if (alarmNotification->monitoredAttributes[i].attributeType < > SA_NTF_VALUE_UINT8 || > - alarmNotification->monitoredAttributes[i].attributeType > > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid monitoredAttributes type value"); > - return SA_AIS_ERR_INVALID_PARAM; > - } > - } > - > - for (i = 0; i < alarmNotification->numProposedRepairActions; i++) { > - if (alarmNotification- > >proposedRepairActions[i].actionValueType < SA_NTF_VALUE_UINT8 || > - alarmNotification- > >proposedRepairActions[i].actionValueType > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid proposedRepairActions type > value"); > - return SA_AIS_ERR_INVALID_PARAM; > - } > - } > - > - if(alarmNotification->thresholdInformation->thresholdValueType < > SA_NTF_VALUE_UINT8 || > - alarmNotification->thresholdInformation- > >thresholdValueType > SA_NTF_VALUE_ARRAY) { > + if(checkNtfValueTypeRange(notification->thresholdInformation- > >thresholdValueType) != SA_AIS_OK) { > TRACE_1("Invalid thresholdInformation type value"); > return SA_AIS_ERR_INVALID_PARAM; > } > > - if (*alarmNotification->notificationHeader.eventType < > SA_NTF_ALARM_NOTIFICATIONS_START || > - *alarmNotification->notificationHeader.eventType > > SA_NTF_ALARM_ENVIRONMENT) { > - TRACE_1("Invalid eventType value = %d", > (int)*alarmNotification->notificationHeader.eventType); > + if (*notification->notificationHeader.eventType < > SA_NTF_ALARM_NOTIFICATIONS_START || > + *notification->notificationHeader.eventType > > SA_NTF_ALARM_ENVIRONMENT) { > + TRACE_1("Invalid eventType value = %d", (int)*notification- > >notificationHeader.eventType); > return SA_AIS_ERR_INVALID_PARAM; > } > > - return checkHeader(&alarmNotification->notificationHeader); > + return checkHeader(pvdata, ¬ification->notificationHeader); > } > > -static SaAisErrorT > checkSecurityAlarmParameters(SaNtfSecurityAlarmNotificationT > *notification) > +static SaAisErrorT > checkSecurityAlarmParameters(ntfa_notification_hdl_rec_t *not_hdl_rec, > v_data *pvdata) > { > - > + SaNtfSecurityAlarmNotificationT *notification = ¬_hdl_rec- > >ntfNotification.ntfSecurityAlarmNotification; > if (*notification->notificationHeader.eventType < > SA_NTF_SECURITY_ALARM_NOTIFICATIONS_START || > *notification->notificationHeader.eventType > > SA_NTF_TIME_VIOLATION) { > TRACE_1("Invalid eventType value"); > @@ -399,23 +581,39 @@ static SaAisErrorT checkSecurityAlarmPar > return SA_AIS_ERR_INVALID_PARAM; > } > > - if (notification->securityAlarmDetector->valueType < > SA_NTF_VALUE_UINT8 || > - notification->securityAlarmDetector->valueType > > SA_NTF_VALUE_ARRAY) { > - TRACE_1("Invalid securityAlarmDetector valueType"); > + if (checkNtfValue(pvdata > + , notification- > >securityAlarmDetector->valueType > + , ¬ification- > >securityAlarmDetector->value) != SA_AIS_OK) { > + TRACE_1("Invalid securityAlarmDetector"); > return SA_AIS_ERR_INVALID_PARAM; > } > > + if (checkNtfValue(pvdata > + , notification->serviceUser- > >valueType > + , ¬ification->serviceUser- > >value) != SA_AIS_OK) { > + TRACE_1("Invalid serviceUser"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > + if (checkNtfValue(pvdata > + , notification- > >serviceProvider->valueType > + , ¬ification- > >serviceProvider->value) != SA_AIS_OK) { > + TRACE_1("Invalid serviceProvider"); > + return SA_AIS_ERR_INVALID_PARAM; > + } > + > if (*notification->severity < SA_NTF_SEVERITY_CLEARED || > *notification->severity > SA_NTF_SEVERITY_CRITICAL) { > TRACE_1("Invalid Severity value"); > return SA_AIS_ERR_INVALID_PARAM; > } > > - return checkHeader(¬ification->notificationHeader); > + return checkHeader(pvdata, ¬ification->notificationHeader); > } > > -static SaAisErrorT > checkStateChangeParameters(SaNtfStateChangeNotificationT *notification) > +static SaAisErrorT > checkStateChangeParameters(ntfa_notification_hdl_rec_t *not_hdl_rec, > v_data *pvdata) > { > int i; > + SaNtfStateChangeNotificationT *notification = ¬_hdl_rec- > >ntfNotification.ntfStateChangeNotification; > if (*notification->notificationHeader.eventType < > SA_NTF_STATE_CHANGE_NOTIFICATIONS_START || > (*notification->notificationHeader.eventType > > SA_NTF_OBJECT_STATE_CHANGE && > *notification->notificationHeader.eventType < > SA_NTF_MISCELLANEOUS_NOTIFICATIONS_START) || > @@ -435,12 +633,13 @@ static SaAisErrorT checkStateChangeParam > if (sp != SA_FALSE && sp != SA_TRUE) > return SA_AIS_ERR_INVALID_PARAM; > } > - return checkHeader(¬ification->notificationHeader); > + return checkHeader(pvdata, ¬ification->notificationHeader); > } > > -static SaAisErrorT > checkAttributeChangeParameters(SaNtfAttributeChangeNotificationT > *notification) > +static SaAisErrorT > checkAttributeChangeParameters(ntfa_notification_hdl_rec_t > *not_hdl_rec, v_data *pvdata) > { > int i; > + SaNtfAttributeChangeNotificationT *notification = ¬_hdl_rec- > >ntfNotification.ntfAttributeChangeNotification; > if (*notification->notificationHeader.eventType < > SA_NTF_ATTRIBUTE_NOTIFICATIONS_START || > *notification->notificationHeader.eventType > > SA_NTF_ATTRIBUTE_RESET) { > TRACE_1("Invalid eventType value"); > @@ -457,18 +656,17 @@ static SaAisErrorT checkAttributeChangeP > SaBoolT sp = notification- > >changedAttributes[i].oldAttributePresent; > if (sp != SA_FALSE && sp != SA_TRUE) > return SA_AIS_ERR_INVALID_PARAM; > - if(notification->changedAttributes[i].attributeType < > SA_NTF_VALUE_UINT8 || > - notification->changedAttributes[i].attributeType > > SA_NTF_VALUE_ARRAY) > + if(checkNtfValueTypeRange(notification- > >changedAttributes[i].attributeType) != SA_AIS_OK) > return SA_AIS_ERR_INVALID_PARAM; > } > - return checkHeader(¬ification->notificationHeader); > + return checkHeader(pvdata, ¬ification->notificationHeader); > } > > -static SaAisErrorT > checkObjectCreateDeleteParameters(SaNtfObjectCreateDeleteNotificationT > *notification) > +static SaAisErrorT > checkObjectCreateDeleteParameters(ntfa_notification_hdl_rec_t > *not_hdl_rec, v_data *pvdata) > { > > int i = 0; > - > + SaNtfObjectCreateDeleteNotificationT *notification = ¬_hdl_rec- > >ntfNotification.ntfObjectCreateDeleteNotification; > if (*notification->notificationHeader.eventType < > SA_NTF_OBJECT_NOTIFICATIONS_START || > *notification->notificationHeader.eventType > > SA_NTF_OBJECT_DELETION) { > TRACE_1("Invalid eventType value"); > @@ -482,14 +680,13 @@ static SaAisErrorT checkObjectCreateDele > } > > for (i = 0; i < notification->numAttributes; i++) { > - if (notification->objectAttributes[i].attributeType < > SA_NTF_VALUE_UINT8 || > - notification->objectAttributes[i].attributeType > > SA_NTF_VALUE_ARRAY) { > + if (checkNtfValueTypeRange(notification- > >objectAttributes[i].attributeType) != SA_AIS_OK) { > TRACE_1("Invalid attributeType value"); > return SA_AIS_ERR_INVALID_PARAM; > } > } > > - return checkHeader(¬ification->notificationHeader); > + return checkHeader(pvdata, ¬ification->notificationHeader); > } > > /** > @@ -1228,7 +1425,8 @@ SaAisErrorT saNtfNotificationSend(SaNtfN > msg.info.api_info.param.send_notification = send_param; > send_param->client_id = client_rec->ntfs_client_id; > send_param->notificationType = notification_hdl_rec- > >ntfNotificationType; > - > + ntfsv_v_data_cp(&send_param->variable_data, > ¬ification_hdl_rec->variable_data); > + > osafassert(pthread_mutex_lock(&ntfa_cb.cb_lock) == 0); > /* Check parameters, depending on type */ > switch (notification_hdl_rec->ntfNotificationType) { > @@ -1237,36 +1435,34 @@ SaAisErrorT saNtfNotificationSend(SaNtfN > /* TODO: assign send_param for all */ > send_param->notification.alarm = notification_hdl_rec- > >ntfNotification.ntfAlarmNotification; > ntfHeader = ¬ification_hdl_rec- > >ntfNotification.ntfAlarmNotification.notificationHeader; > - rc = checkAlarmParameters(¬ification_hdl_rec- > >ntfNotification.ntfAlarmNotification); > + rc = checkAlarmParameters(notification_hdl_rec, > &send_param->variable_data); > break; > case SA_NTF_TYPE_SECURITY_ALARM: > TRACE_1("Checking Security Alarm Notification Parameters"); > send_param->notification.securityAlarm = > notification_hdl_rec- > >ntfNotification.ntfSecurityAlarmNotification; > ntfHeader = ¬ification_hdl_rec- > >ntfNotification.ntfSecurityAlarmNotification.notificationHeader; > - rc = checkSecurityAlarmParameters(¬ification_hdl_rec- > >ntfNotification.ntfSecurityAlarmNotification); > + rc = checkSecurityAlarmParameters(notification_hdl_rec, > &send_param->variable_data); > break; > case SA_NTF_TYPE_STATE_CHANGE: > TRACE_1("Checking State Change Notification Parameters"); > send_param->notification.stateChange = > notification_hdl_rec->ntfNotification.ntfStateChangeNotification; > ntfHeader = ¬ification_hdl_rec- > >ntfNotification.ntfStateChangeNotification.notificationHeader; > - rc = checkStateChangeParameters(¬ification_hdl_rec- > >ntfNotification.ntfStateChangeNotification); > + rc = checkStateChangeParameters(notification_hdl_rec, > &send_param->variable_data); > break; > case SA_NTF_TYPE_ATTRIBUTE_CHANGE: > TRACE_1("Checking Attribute Change Notification > Parameters"); > send_param->notification.attributeChange = > notification_hdl_rec- > >ntfNotification.ntfAttributeChangeNotification; > ntfHeader = ¬ification_hdl_rec- > >ntfNotification.ntfAttributeChangeNotification.notificationHeader; > - rc = > checkAttributeChangeParameters(¬ification_hdl_rec->ntfNotification. > - > ntfAttributeChangeNotification); > + rc = checkAttributeChangeParameters(notification_hdl_rec, > &send_param->variable_data); > break; > case SA_NTF_TYPE_OBJECT_CREATE_DELETE: > TRACE_1("Checking Object Create/Delete Notification > Parameters"); > send_param->notification.objectCreateDelete = > notification_hdl_rec- > >ntfNotification.ntfObjectCreateDeleteNotification; > ntfHeader = ¬ification_hdl_rec- > >ntfNotification.ntfObjectCreateDeleteNotification.notificationHeader; > - rc = > checkObjectCreateDeleteParameters(¬ification_hdl_rec- > >ntfNotification. > - > ntfObjectCreateDeleteNotification); > + rc = > checkObjectCreateDeleteParameters(notification_hdl_rec, &send_param- > >variable_data); > break; > default: > TRACE_1("Unkown notification type"); > @@ -1278,7 +1474,6 @@ SaAisErrorT saNtfNotificationSend(SaNtfN > TRACE_1("Invalid parameter"); > goto done_give_hdls; > } > - > rc = fillSendStruct(ntfHeader, send_param); > if (rc != SA_AIS_OK) { > goto done_give_hdls; > @@ -1290,33 +1485,7 @@ SaAisErrorT saNtfNotificationSend(SaNtfN > rc = SA_AIS_ERR_TRY_AGAIN; > goto done_give_hdls; > } > - send_param->variable_data = notification_hdl_rec->variable_data; > - SaUint16T i = 0; > - for(i=0 ; i < ntfHeader->numAdditionalInfo ; i++ ) { > - if (ntfHeader->additionalInfo[i].infoType == > SA_NTF_VALUE_LDAP_NAME) { > - SaNameT *ptr = send_param->variable_data.p_base > + > - ntfHeader- > >additionalInfo[i].infoValue.ptrVal.dataOffset; > - if (!ntfsv_sanamet_is_valid(ptr)) { > - rc = SA_AIS_ERR_INVALID_PARAM; > - goto done_give_hdls; > - } > - if (osaf_is_an_extended_name(ptr)) { > - size_t length_to_copy; > - void *p; > - if (ntfHeader- > >additionalInfo[i].infoValue.ptrVal.dataSize > - < > (osaf_extended_name_length(ptr) + 2)) { > - LOG_ER("The allocated memory is > not large enough," > - " the object will be > truncated (%s)" > - , > osaf_extended_name_borrow(ptr)); > - length_to_copy = ntfHeader- > >additionalInfo[i].infoValue.ptrVal.dataSize - 2; > - } else > - length_to_copy = > osaf_extended_name_length(ptr); > - p = send_param->variable_data.p_base + > - ntfHeader- > >additionalInfo[i].infoValue.ptrVal.dataOffset + 2; > - memcpy(p, > osaf_extended_name_borrow(ptr), length_to_copy); > - } > - } > - } > + > osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0); > /* Send a sync MDS message to obtain a notification id */ > mds_rc = ntfa_mds_msg_sync_send(&ntfa_cb, &msg, &o_msg, > timeout); > @@ -1392,6 +1561,8 @@ SaAisErrorT saNtfNotificationSend(SaNtfN > done_give_hdl: > ncshm_give_hdl(notificationHandle); > err_free: > + if (send_param->variable_data.p_base != NULL) > + free(send_param->variable_data.p_base); > free(send_param); > done: > TRACE_LEAVE(); > @@ -1976,6 +2147,7 @@ SaAisErrorT saNtfPtrValGet(SaNtfNotifica > unsigned int client_handle; > ntfa_client_hdl_rec_t *client_rec; > ntfa_notification_hdl_rec_t *notification_hdl_rec; > + SaNtfValueTypeT type; > TRACE_ENTER(); > if (notificationHandle == 0) { > rc = SA_AIS_ERR_BAD_HANDLE; > @@ -1994,6 +2166,30 @@ SaAisErrorT saNtfPtrValGet(SaNtfNotifica > goto done; > } > > + if (getTypeFromValue(notification_hdl_rec, value, &type) != > SA_AIS_OK) { > + TRACE("SaNtfValueT@value is not found in notification"); > + rc = SA_AIS_ERR_INVALID_PARAM; > + goto done_give_hdl; > + } > + > + if (type != SA_NTF_VALUE_LDAP_NAME && type != > SA_NTF_VALUE_STRING && > + type != SA_NTF_VALUE_IPADDRESS && type != > SA_NTF_VALUE_BINARY) { > + TRACE("type(%d) of SaNtfValueT@value is not valid for > saNtfPtrValGet()", type); > + rc = SA_AIS_ERR_INVALID_PARAM; > + goto done_give_hdl; > + } > + > + if (type == SA_NTF_VALUE_LDAP_NAME) { > + SaNameT name; > + > osaf_extended_name_lend((SaConstStringT)notification_hdl_rec- > >variable_data.p_base > + + > value->ptrVal.dataOffset + 2, &name); > + if (!ntfsv_sanamet_is_valid(&name)) { > + TRACE("Invalid value of type(%d)", type); > + rc = SA_AIS_ERR_NAME_TOO_LONG; > + goto done_give_hdl; > + } > + } > + > client_handle = notification_hdl_rec->parent_hdl->local_hdl; > /* retrieve client hdl rec */ > client_rec = ncshm_take_hdl(NCS_SERVICE_ID_NTFA, > client_handle); > @@ -2854,7 +3050,7 @@ SaAisErrorT saNtfNotificationReadNext(Sa > uint32_t timeout = NTFS_WAIT_TIME; > ntfsv_send_not_req_t *read_not = NULL; > ntfa_notification_hdl_rec_t *notification_hdl_rec = NULL; > - > + SaNtfNotificationHeaderT *read_not_header = NULL; > TRACE_ENTER(); > > if(searchDirection < SA_NTF_SEARCH_OLDER || searchDirection > > SA_NTF_SEARCH_YOUNGER) { > @@ -2902,26 +3098,47 @@ SaAisErrorT saNtfNotificationReadNext(Sa > goto done_give_hdls; > } > > - /* Send a sync MDS message */ > - rc = ntfa_mds_msg_sync_send(&ntfa_cb, &msg, &o_msg, timeout); > - if (rc != NCSCC_RC_SUCCESS) { > - rc = SA_AIS_ERR_TRY_AGAIN; > - goto done_give_hdls; > - } > - > - osafassert(o_msg != NULL); > - if (SA_AIS_OK != o_msg->info.api_resp_info.rc) { > - rc = o_msg->info.api_resp_info.rc; > - TRACE("error: response msg rc = %d", rc); > - goto done_give_hdls; > - } > - if (o_msg->info.api_resp_info.type != NTFSV_READ_NEXT_RSP) { > - TRACE("msg type (%d) failed", (int)o_msg- > >info.api_resp_info.type); > - rc = SA_AIS_ERR_LIBRARY; > - goto done_give_hdls; > - } > - > - read_not = o_msg- > >info.api_resp_info.param.read_next_rsp.readNotification; > + do { > + /* Send a sync MDS message */ > + rc = ntfa_mds_msg_sync_send(&ntfa_cb, &msg, &o_msg, > timeout); > + if (rc != NCSCC_RC_SUCCESS) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto done_give_hdls; > + } > + > + osafassert(o_msg != NULL); > + if (SA_AIS_OK != o_msg->info.api_resp_info.rc) { > + rc = o_msg->info.api_resp_info.rc; > + TRACE("error: response msg rc = %d", rc); > + goto done_give_hdls; > + } > + if (o_msg->info.api_resp_info.type != > NTFSV_READ_NEXT_RSP) { > + TRACE("msg type (%d) failed", (int)o_msg- > >info.api_resp_info.type); > + rc = SA_AIS_ERR_LIBRARY; > + goto done_give_hdls; > + } > + > + read_not = o_msg- > >info.api_resp_info.param.read_next_rsp.readNotification; > + if (read_not->notificationType == SA_NTF_TYPE_ALARM) > + read_not_header = &read_not- > >notification.alarm.notificationHeader; > + else if (read_not->notificationType == > SA_NTF_TYPE_SECURITY_ALARM) > + read_not_header = &read_not- > >notification.securityAlarm.notificationHeader; > + else { > + TRACE_1("Notification type (%d) is not alarm!", > (int)read_not->notificationType); > + rc = SA_AIS_ERR_NOT_SUPPORTED; > + goto done_give_hdls; > + } > + > + /* Skip notification having longDns if this is unadapted > longDns consumer, > + * continue reading next. Otherwise, any return code differs > SA_AIS_OK may stop > + * the consumer reading then miss out the remaining > notifications (shortDns) > + */ > + if (!ntfsv_sanamet_is_valid(read_not_header- > >notificationObject) > + || !ntfsv_sanamet_is_valid(read_not_header- > >notifyingObject)) { > + ntfa_msg_destroy(o_msg); > + } else > + break; > + } while (1); > > /* Only alarm supported */ > if (read_not->notificationType == SA_NTF_TYPE_ALARM) { > @@ -2996,9 +3213,6 @@ SaAisErrorT saNtfNotificationReadNext(Sa > ntfsv_copy_ntf_security_alarm(¬ification- > >notification.securityAlarmNotification, > &read_not- > >notification.securityAlarm); > osafassert(pthread_mutex_unlock(&ntfa_cb.cb_lock) == 0); > - } else { > - TRACE_1("Notification type (%d) is not alarm!", > (int)read_not->notificationType); > - rc = SA_AIS_ERR_NOT_SUPPORTED; > } > > done_give_hdls: > diff --git a/osaf/libs/agents/saf/ntfa/ntfa_util.c > b/osaf/libs/agents/saf/ntfa/ntfa_util.c > --- a/osaf/libs/agents/saf/ntfa/ntfa_util.c > +++ b/osaf/libs/agents/saf/ntfa/ntfa_util.c > @@ -137,10 +137,16 @@ static SaAisErrorT ntfa_alloc_callback_n > SaAisErrorT rc = SA_AIS_OK; > ntfa_notification_hdl_rec_t *notification_hdl_rec; > notification->notificationType = not_cbk->notificationType; > - > + SaNtfNotificationHeaderT *not_cbk_header; > switch (not_cbk->notificationType) { > case SA_NTF_TYPE_OBJECT_CREATE_DELETE: > TRACE_2("type: SA_NTF_TYPE_OBJECT_CREATE_DELETE"); > + not_cbk_header = ¬_cbk- > >notification.objectCreateDelete.notificationHeader; > + if (!ntfsv_sanamet_is_valid(not_cbk_header- > >notificationObject) > + || !ntfsv_sanamet_is_valid(not_cbk_header- > >notifyingObject)) { > + rc = SA_AIS_ERR_NAME_TOO_LONG; > + break; > + } > rc = saNtfObjectCreateDeleteNotificationAllocate(hdl_rec- > >local_hdl, > > ¬ification->notification. > > objectCreateDeleteNotification, > @@ -175,6 +181,12 @@ static SaAisErrorT ntfa_alloc_callback_n > break; > case SA_NTF_TYPE_ATTRIBUTE_CHANGE: > TRACE_2("type: SA_NTF_TYPE_ATTRIBUTE_CHANGE"); > + not_cbk_header = ¬_cbk- > >notification.attributeChange.notificationHeader; > + if (!ntfsv_sanamet_is_valid(not_cbk_header- > >notificationObject) > + || !ntfsv_sanamet_is_valid(not_cbk_header- > >notifyingObject)) { > + rc = SA_AIS_ERR_NAME_TOO_LONG; > + break; > + } > rc = saNtfAttributeChangeNotificationAllocate(hdl_rec- > >local_hdl, > ¬ification- > >notification.attributeChangeNotification, > not_cbk- > >notification.attributeChange.notificationHeader. > @@ -208,6 +220,12 @@ static SaAisErrorT ntfa_alloc_callback_n > break; > case SA_NTF_TYPE_STATE_CHANGE: > TRACE_2("type: SA_NTF_TYPE_STATE_CHANGE"); > + not_cbk_header = ¬_cbk- > >notification.stateChange.notificationHeader; > + if (!ntfsv_sanamet_is_valid(not_cbk_header- > >notificationObject) > + || !ntfsv_sanamet_is_valid(not_cbk_header- > >notifyingObject)) { > + rc = SA_AIS_ERR_NAME_TOO_LONG; > + break; > + } > rc = saNtfStateChangeNotificationAllocate(hdl_rec- > >local_hdl, > ¬ification- > >notification.stateChangeNotification, > not_cbk- > >notification.stateChange.notificationHeader. > @@ -240,6 +258,13 @@ static SaAisErrorT ntfa_alloc_callback_n > } > break; > case SA_NTF_TYPE_ALARM: > + TRACE_2("type: SA_NTF_TYPE_ALARM"); > + not_cbk_header = ¬_cbk- > >notification.alarm.notificationHeader; > + if (!ntfsv_sanamet_is_valid(not_cbk_header- > >notificationObject) > + || !ntfsv_sanamet_is_valid(not_cbk_header- > >notifyingObject)) { > + rc = SA_AIS_ERR_NAME_TOO_LONG; > + break; > + } > rc = saNtfAlarmNotificationAllocate(hdl_rec->local_hdl, > ¬ification- > >notification.alarmNotification, > not_cbk- > >notification.alarm. > @@ -273,6 +298,12 @@ static SaAisErrorT ntfa_alloc_callback_n > break; > case SA_NTF_TYPE_SECURITY_ALARM: > TRACE_2("type: SA_NTF_TYPE_SECURITY_ALARM"); > + not_cbk_header = ¬_cbk- > >notification.securityAlarm.notificationHeader; > + if (!ntfsv_sanamet_is_valid(not_cbk_header- > >notificationObject) > + || !ntfsv_sanamet_is_valid(not_cbk_header- > >notifyingObject)) { > + rc = SA_AIS_ERR_NAME_TOO_LONG; > + break; > + } > rc = saNtfSecurityAlarmNotificationAllocate(hdl_rec- > >local_hdl, > ¬ification- > >notification.securityAlarmNotification, > not_cbk- > >notification.securityAlarm.notificationHeader. > @@ -347,6 +378,12 @@ static SaAisErrorT ntfa_hdl_cbk_rec_prc( > rc = > ntfa_alloc_callback_notification(notification, > cbk_info- > >param.notification_cbk, hdl_rec); > if (rc != SA_AIS_OK) { > + /* Returned code > ERR_NAME_TOO_LONG is due to receiving longDn notification > + * in unadapted-longDN subscriber, > need to return OK here in order to > + * avoid consumer exit() in case that > saNtfDispatch() returns non-OK > + */ > + if (rc == > SA_AIS_ERR_NAME_TOO_LONG) > + rc = SA_AIS_OK; > /* not in handle struct */ > free(notification); > goto done; > diff --git a/osaf/libs/common/ntfsv/ntfsv_mem.c > b/osaf/libs/common/ntfsv/ntfsv_mem.c > --- a/osaf/libs/common/ntfsv/ntfsv_mem.c > +++ b/osaf/libs/common/ntfsv/ntfsv_mem.c > @@ -822,6 +822,7 @@ SaAisErrorT ntfsv_ptr_val_alloc(v_data * > nv->ptrVal.dataOffset = vd->size; > nv->ptrVal.dataSize = data_size; > *data_ptr = vd->p_base + vd->size; > + memset(*data_ptr, 0, data_size); > vd->size += data_size; > } else { > TRACE("SA_AIS_ERR_NO_SPACE\n"); ------------------------------------------------------------------------------ Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
