Re: [devel] [PATCH 1 of 1] imm: Ensure strings sent to IMM server are null terminated [#1277]

[Anders B. <and...@er...>]

Ack, not tested.

I do have some minor improvement comments, but I don't want to delay this fix for RC1 4.6.
So these improvements could be done as a separate enhancement for (4.7).

(1) We could have a small boolean inline function that verifies if an SaNameT value is null-terminated, called something like: 
	 osaf_extended_name_nulterm(..).
Instead of always copying the string, we would just use osaf_extended_name_borrow() if the SaNameT value passed the nullterm test.
Only if the SaNameT value fails the nullterm test would we need to fix null termination using a copy.

(2) The copy (when needed) should in principle be possible to stack allocated as a char array of the right size inside a code block.
The array would be zero sized if no copy is needed (the original value was null terminated). That code block would encompass the  entire
sending of the string to the server.  The string to be sent is either the stack allocated buffer or the pointer returned from borrow if the 
stack buffer has size zero.  

/AndersBj



-----Original Message-----
From: Hung Nguyen [mailto:hun...@de...] 
Sent: den 23 mars 2015 14:18
To: Anders Björnerstedt
Cc: ope...@li...
Subject: [PATCH 1 of 1] imm: Ensure strings sent to IMM server are null terminated [#1277]

 osaf/libs/agents/saf/imma/imma_oi_api.c |  13 ++++++++++++-  osaf/libs/agents/saf/imma/imma_om_api.c |  13 ++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)


Ensure strings sent to IMM server are null terminated.

diff --git a/osaf/libs/agents/saf/imma/imma_oi_api.c b/osaf/libs/agents/saf/imma/imma_oi_api.c
--- a/osaf/libs/agents/saf/imma/imma_oi_api.c
+++ b/osaf/libs/agents/saf/imma/imma_oi_api.c
@@ -2839,7 +2839,13 @@ static SaAisErrorT rt_object_create_comm
 		size_t parentNameLen = 0;
 		if (parentName && (parentNameLen = osaf_extended_name_length(parentName)) > 0) {
 			evt.info.immnd.info.objCreate.parentOrObjectDn.size = parentNameLen + 1;
-			evt.info.immnd.info.objCreate.parentOrObjectDn.buf = (char *)osaf_extended_name_borrow(parentName);
+			evt.info.immnd.info.objCreate.parentOrObjectDn.buf = (char*) malloc((parentNameLen + 1) * sizeof(char));
+			if (!evt.info.immnd.info.objCreate.parentOrObjectDn.buf) {
+				rc = SA_AIS_ERR_NO_MEMORY;
+				goto mds_send_fail;
+			}
+			memcpy(evt.info.immnd.info.objCreate.parentOrObjectDn.buf, osaf_extended_name_borrow(parentName), parentNameLen);
+			evt.info.immnd.info.objCreate.parentOrObjectDn.buf[parentNameLen] = 
+'\0';
 		} else {
 			evt.info.immnd.info.objCreate.parentOrObjectDn.size = 0;
 			evt.info.immnd.info.objCreate.parentOrObjectDn.buf = NULL; @@ -2988,6 +2994,11 @@ static SaAisErrorT rt_object_create_comm
 		evt.info.immnd.info.objCreate.className.buf = NULL;
 	}
 
+	if (parentName && evt.info.immnd.info.objCreate.parentOrObjectDn.buf) {
+		free(evt.info.immnd.info.objCreate.parentOrObjectDn.buf);
+		evt.info.immnd.info.objCreate.parentOrObjectDn.buf = NULL;
+	}
+
 	while (evt.info.immnd.info.objCreate.attrValues) {
 		IMMSV_ATTR_VALUES_LIST *p = evt.info.immnd.info.objCreate.attrValues;
 		evt.info.immnd.info.objCreate.attrValues = p->next; diff --git a/osaf/libs/agents/saf/imma/imma_om_api.c b/osaf/libs/agents/saf/imma/imma_om_api.c
--- a/osaf/libs/agents/saf/imma/imma_om_api.c
+++ b/osaf/libs/agents/saf/imma/imma_om_api.c
@@ -1838,7 +1838,13 @@ static SaAisErrorT ccb_object_create_com
 		}
 
 		evt.info.immnd.info.objCreate.parentOrObjectDn.size = parentNameLength + 1;
-		evt.info.immnd.info.objCreate.parentOrObjectDn.buf = (char *)osaf_extended_name_borrow(parentName);
+		evt.info.immnd.info.objCreate.parentOrObjectDn.buf = (char*) malloc((parentNameLength + 1) * sizeof(char));
+		if (!evt.info.immnd.info.objCreate.parentOrObjectDn.buf) {
+			rc = SA_AIS_ERR_NO_MEMORY;
+			goto mds_send_fail;
+		}
+		memcpy(evt.info.immnd.info.objCreate.parentOrObjectDn.buf, osaf_extended_name_borrow(parentName), parentNameLength);
+		evt.info.immnd.info.objCreate.parentOrObjectDn.buf[parentNameLength] 
+= '\0';
 	} else if (objectName) {
 		size_t objectNameLength = strlen(objectName);
 		if(!osaf_is_extended_names_enabled() && objectNameLength >= SA_MAX_UNEXTENDED_NAME_LENGTH) { @@ -1987,6 +1993,11 @@ static SaAisErrorT ccb_object_create_com
 		evt.info.immnd.info.objCreate.className.buf = NULL;
 	}
 
+	if (parentName && evt.info.immnd.info.objCreate.parentOrObjectDn.buf) {
+		free(evt.info.immnd.info.objCreate.parentOrObjectDn.buf);
+		evt.info.immnd.info.objCreate.parentOrObjectDn.buf = NULL;
+	}
+
 	while (evt.info.immnd.info.objCreate.attrValues) {
 		IMMSV_ATTR_VALUES_LIST *p = evt.info.immnd.info.objCreate.attrValues;
 		evt.info.immnd.info.objCreate.attrValues = p->next;