Good catch on this programming error. I have one comment inline re code
style for our software - could you fix and resubmit?
Thanks
-steve
On 02/13/2011 08:43 PM, Russell Bryant wrote:
> This patch ensures that the resulting string is null terminated after a
> call to strncpy().
>
> ---
> cts/agents/cpg_test_agent.c | 2 ++
> exec/logsys.c | 7 +++++--
> exec/util.c | 3 ++-
> test/sa_error.c | 2 ++
> tools/corosync-notifyd.c | 9 ++++++---
> tools/corosync-objctl.c | 9 ++++++---
> 6 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/cts/agents/cpg_test_agent.c b/cts/agents/cpg_test_agent.c
> index e94cb6b..1763a1c 100644
> --- a/cts/agents/cpg_test_agent.c
> +++ b/cts/agents/cpg_test_agent.c
> @@ -124,6 +124,8 @@ 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 72d3f68..2dad6dd 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..f1681ff 100644
> --- a/test/sa_error.c
> +++ b/test/sa_error.c
> @@ -46,6 +46,8 @@ 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';
if (len > 0) {
str[len - 1] = '\0';
}
We require {} around one liner conditional execution statements so
people adding code don't inadvertently break the software when adding
code. This is how the at&t network failed in the 90s. (or was it 80s)
for multiple days.
Regards
-steve
> 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
