Skip to content

Commit

Permalink
plugin: use strlcpy()
Browse files Browse the repository at this point in the history
Problem: strncpy() could leave a string unterminated if not
zero-initialized, and the keys for the info[2] array in main.c
not zero-initalized.

Switch strlcpy() wherever strncpy() is used.

Use sizeof() where appropriate instead of pmix "MAX" #defines,
since that removes all ambiguity about whether the #define includes
space for the NULL or not (it does not, but I have to keep checking).
  • Loading branch information
garlick committed Nov 11, 2021
1 parent c3a8c92 commit 343215a
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 11 deletions.
6 changes: 4 additions & 2 deletions src/shell/plugins/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ pmix_la_LIBADD = \
$(FLUX_IDSET_LIBS) \
$(FLUX_HOSTLIST_LIBS) \
$(JANSSON_LIBS) \
$(top_builddir)/src/common/libccan/libccan.la
$(top_builddir)/src/common/libccan/libccan.la \
$(top_builddir)/src/common/libutil/libutil.la
pmix_la_LDFLAGS = \
$(AM_LDFLAGS) \
$(fluxplugin_ldflags) \
Expand All @@ -58,7 +59,8 @@ TESTS = \

test_ldadd = \
$(top_builddir)/src/common/libtap/libtap.la \
$(top_builddir)/src/common/libccan/libccan.la
$(top_builddir)/src/common/libccan/libccan.la \
$(top_builddir)/src/common/libutil/libutil.la

test_ldflags = \
-no-install
Expand Down
5 changes: 3 additions & 2 deletions src/shell/plugins/codec.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <pmix_server.h>

#include "src/common/libccan/ccan/base64/base64.h"
#include "src/common/libutil/strlcpy.h"

#include "codec.h"

Expand Down Expand Up @@ -119,7 +120,7 @@ int codec_proc_decode (json_t *o, pmix_proc_t *proc)
"rank", &rank) < 0)
return -1;
proc->rank = rank;
strncpy (proc->nspace, nspace, PMIX_MAX_NSLEN);
strlcpy (proc->nspace, nspace, sizeof (proc->nspace));
proc->nspace[PMIX_MAX_NSLEN] = '\0';
return 0;
}
Expand Down Expand Up @@ -399,7 +400,7 @@ int codec_info_decode (json_t *o, pmix_info_t *info)
return -1;
if (codec_value_decode (xvalue, &info->value) < 0) // allocs mem
return -1;
strncpy (info->key, key, PMIX_MAX_KEYLEN);
strlcpy (info->key, key, sizeof (info->key));
info->key[PMIX_MAX_KEYLEN] = '\0';
return 0;
}
Expand Down
10 changes: 6 additions & 4 deletions src/shell/plugins/infovec.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include <errno.h>
#include <pmix.h>

#include "src/common/libutil/strlcpy.h"

#include "infovec.h"

#define INFOVEC_CHUNK 8
Expand Down Expand Up @@ -52,7 +54,7 @@ int infovec_set_str_new (struct infovec *iv, const char *key, char *val)
}
if (!(info = alloc_slot (iv)))
return -1;
strncpy (info->key, key, PMIX_MAX_KEYLEN);
strlcpy (info->key, key, sizeof (info->key));
info->value.type = PMIX_STRING;
info->value.data.string = val;
return 0;
Expand Down Expand Up @@ -86,7 +88,7 @@ int infovec_set_u32 (struct infovec *iv, const char *key, uint32_t value)
}
if (!(info = alloc_slot (iv)))
return -1;
strncpy (info->key, key, PMIX_MAX_KEYLEN);
strlcpy (info->key, key, sizeof (info->key));
info->value.type = PMIX_UINT32;
info->value.data.uint32 = value;
return 0;
Expand All @@ -102,7 +104,7 @@ int infovec_set_bool (struct infovec *iv, const char *key, bool value)
}
if (!(info = alloc_slot (iv)))
return -1;
strncpy (info->key, key, PMIX_MAX_KEYLEN);
strlcpy (info->key, key, sizeof (info->key));
info->value.type = PMIX_BOOL;
info->value.data.flag = value;
return 0;
Expand All @@ -120,7 +122,7 @@ int infovec_set_rank (struct infovec *iv,
}
if (!(info = alloc_slot (iv)))
return -1;
strncpy (info->key, key, PMIX_MAX_KEYLEN);
strlcpy (info->key, key, sizeof (info->key));
info->value.type = PMIX_PROC_RANK;
info->value.data.rank = value;
return 0;
Expand Down
4 changes: 3 additions & 1 deletion src/shell/plugins/interthread.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include <jansson.h>
#include <flux/core.h>

#include "src/common/libutil/strlcpy.h"

#include "interthread.h"

#define MAX_HANDLERS 32
Expand Down Expand Up @@ -48,7 +50,7 @@ int interthread_register (struct interthread *it,
return -1;
}
handler = &it->handlers[it->handler_count++];
strncpy (handler->topic, topic, sizeof (handler->topic) - 1);
strlcpy (handler->topic, topic, sizeof (handler->topic));
handler->cb = cb;
handler->arg = arg;
return 0;
Expand Down
6 changes: 4 additions & 2 deletions src/shell/plugins/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <pmix_server.h>
#include <pmix.h>

#include "src/common/libutil/strlcpy.h"

#include "infovec.h"
#include "maps.h"
#include "interthread.h"
Expand Down Expand Up @@ -189,12 +191,12 @@ static int px_init (flux_plugin_t *p,
return -1;
server_callbacks.direct_modex = dmodex_server_cb;

strncpy (info[0].key, PMIX_SERVER_TMPDIR, PMIX_MAX_KEYLEN);
strlcpy (info[0].key, PMIX_SERVER_TMPDIR, sizeof (info[0].key));
info[0].value.type = PMIX_STRING;
info[0].value.data.string = (char *)px->job_tmpdir;
info[0].flags = 0;

strncpy (info[1].key, PMIX_SERVER_RANK, PMIX_MAX_KEYLEN);
strlcpy (info[1].key, PMIX_SERVER_RANK, sizeof (info[1].key));
info[1].value.type = PMIX_PROC_RANK;
info[1].value.data.rank = px->shell_rank;
info[1].flags = 0;
Expand Down

0 comments on commit 343215a

Please sign in to comment.