Could you amend the description of the commit log to say something like: >From the strcpy(3) man page, the following warning is given: The strncpy() function is similar, except that at most n bytes of src are copied. Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated.
The current corosync code base does not take this warning into account when using strncpy, potentially resulting in non-null terminated strings. Reviewed-by: Steven Dake <[email protected]> On 02/27/2011 07:34 PM, Open SA Forum AIS Services mailing list wrote: > Signed-off-by: Russell Bryant <[email protected]> > --- > cts/agents/cpg_test_agent.c | 3 +++ > exec/logsys.c | 7 +++++-- > exec/util.c | 3 ++- > test/sa_error.c | 3 +++ > tools/corosync-notifyd.c | 9 ++++++--- > tools/corosync-objctl.c | 9 ++++++--- > 6 files changed, 25 insertions(+), 9 deletions(-) > > diff --git a/cts/agents/cpg_test_agent.c b/cts/agents/cpg_test_agent.c > index e94cb6b..a0af9c1 100644 > --- a/cts/agents/cpg_test_agent.c > +++ b/cts/agents/cpg_test_agent.c > @@ -124,6 +124,9 @@ static char* err_status_string (char * buf, size_t > buf_len, msg_status_t status) > strncpy (buf, "UNKNOWN_ERR", buf_len); > break; > } > + if (buf_len > 0) { > + buf[buf_len - 1] = '\0'; > + } > return buf; > } > > diff --git a/exec/logsys.c b/exec/logsys.c > index c678689..30ab9ae 100644 > --- a/exec/logsys.c > +++ b/exec/logsys.c > @@ -936,7 +936,9 @@ static void logsys_subsys_init ( > LOGSYS_LOGGER_INIT_DONE; > } > strncpy (logsys_loggers[subsysid].subsys, subsys, > - LOGSYS_MAX_SUBSYS_NAMELEN); > + sizeof (logsys_loggers[subsysid].subsys)); > + logsys_loggers[subsysid].subsys[ > + sizeof (logsys_loggers[subsysid].subsys) - 1] = '\0'; > } > > /* > @@ -992,7 +994,8 @@ int _logsys_system_setup( > (logsys_loggers[i].init_status == > LOGSYS_LOGGER_NEEDS_INIT)) { > strncpy (tempsubsys, logsys_loggers[i].subsys, > - LOGSYS_MAX_SUBSYS_NAMELEN); > + sizeof (tempsubsys)); > + tempsubsys[sizeof (tempsubsys) - 1] = '\0'; > logsys_subsys_init(tempsubsys, i); > } > } > diff --git a/exec/util.c b/exec/util.c > index 54152c3..a3c2b87 100644 > --- a/exec/util.c > +++ b/exec/util.c > @@ -117,7 +117,8 @@ char *getcs_name_t (cs_name_t *name) > } > > void setcs_name_t (cs_name_t *name, char *str) { > - strncpy ((char *)name->value, str, CS_MAX_NAME_LENGTH); > + strncpy ((char *)name->value, str, sizeof (name->value)); > + ((char *)name->value)[sizeof (name->value) - 1] = '\0'; > if (strlen ((char *)name->value) > CS_MAX_NAME_LENGTH) { > name->length = CS_MAX_NAME_LENGTH; > } else { > diff --git a/test/sa_error.c b/test/sa_error.c > index 679601c..61924c7 100644 > --- a/test/sa_error.c > +++ b/test/sa_error.c > @@ -46,6 +46,9 @@ int get_sa_error(cs_error_t error, char *str, int len) > return -1; > } > strncpy(str, sa_error_list[error], len); > + if (len > 0) { > + str[len - 1] = '\0'; > + } > return 0; > } > > diff --git a/tools/corosync-notifyd.c b/tools/corosync-notifyd.c > index 6e3b45e..9f84cd2 100644 > --- a/tools/corosync-notifyd.c > +++ b/tools/corosync-notifyd.c > @@ -336,7 +336,8 @@ _cs_confdb_find_object (confdb_handle_t handle, > char tmp_name[CS_MAX_NAME_LENGTH]; > cs_error_t res = CS_OK; > > - strncpy (tmp_name, name_pt, CS_MAX_NAME_LENGTH); > + strncpy (tmp_name, name_pt, sizeof (tmp_name)); > + tmp_name[sizeof (tmp_name) - 1] = '\0'; > obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt); > > while (obj_name_pt != NULL) { > @@ -834,7 +835,8 @@ _cs_local_node_info_get(char **nodename, uint32_t *nodeid) > corosync_cfg_finalize(cfg_handle); > if (rc != CS_OK) { > local_nodeid = 0; > - strncpy(local_nodename, "localhost", > CS_MAX_NAME_LENGTH); > + strncpy(local_nodename, "localhost", sizeof > (local_nodename)); > + local_nodename[sizeof (local_nodename) - 1] = '\0'; > } else { > gethostname(local_nodename, CS_MAX_NAME_LENGTH); > } > @@ -1003,7 +1005,8 @@ main(int argc, char *argv[]) > break; > case 'm': > conf[CS_NTF_SNMP] = 1; > - strncpy(snmp_manager_buf, optarg, > CS_MAX_NAME_LENGTH); > + strncpy(snmp_manager_buf, optarg, sizeof > (snmp_manager_buf)); > + snmp_manager_buf[sizeof (snmp_manager_buf) - 1] > = '\0'; > snmp_manager = snmp_manager_buf; > break; > case 'o': > diff --git a/tools/corosync-objctl.c b/tools/corosync-objctl.c > index 2332b79..3052ff0 100644 > --- a/tools/corosync-objctl.c > +++ b/tools/corosync-objctl.c > @@ -406,7 +406,8 @@ static cs_error_t find_object (confdb_handle_t handle, > char tmp_name[OBJ_NAME_SIZE]; > cs_error_t res = CS_OK; > > - strncpy (tmp_name, name_pt, OBJ_NAME_SIZE); > + strncpy (tmp_name, name_pt, sizeof (tmp_name)); > + tmp_name[sizeof (tmp_name) - 1] = '\0'; > obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt); > > while (obj_name_pt != NULL) { > @@ -516,7 +517,8 @@ static void create_object(confdb_handle_t handle, char * > name_pt) > char tmp_name[OBJ_NAME_SIZE]; > cs_error_t res; > > - strncpy (tmp_name, name_pt, OBJ_NAME_SIZE); > + strncpy (tmp_name, name_pt, sizeof (tmp_name)); > + tmp_name[sizeof (tmp_name) - 1] = '\0'; > obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt); > > while (obj_name_pt != NULL) { > @@ -569,7 +571,8 @@ static void create_object_key(confdb_handle_t handle, > char *name_pt) > get_parent_name(name_pt, parent_name); > get_key(name_pt, key_name, key_value); > > - strncpy (tmp_name, parent_name, OBJ_NAME_SIZE); > + strncpy (tmp_name, parent_name, sizeof (tmp_name)); > + tmp_name[sizeof (tmp_name) - 1] = '\0'; > obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt); > > /* _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
