Просмотр исходного кода

cmap: Fix strncpy warning in cmap_iter_next

cmap_iter_next in contrast of it's icmap counterpart copies key name
into user preallocated space. In the worst case, key name may be
CMAP_KEYNAME_MAXLEN, so cmap_iter_next then need CMAP_KEYNAME_MAXLEN +
additional byte to store zero. strncpy was copying only
CMAP_KEYNAME_MAXLEN characters so there was possibility of unterminated
string.

Patch solves this by using memcpy and always add trailing zero.
Documentation was improved suggesting minimum size of keyname buffer to
be CMAP_KEYNAME_MAXLEN + 1.

Also sam and quorumtool were using too short buffer so they are fixed too.

Signed-off-by: Jan Friesse <jfriesse@redhat.com>
Reviewed-by: Christine Caulfield <ccaulfie@redhat.com>
(cherry picked from commit f286388275d50f9e99a2c931a3457865947d3ccd)
Jan Friesse 7 лет назад
Родитель
Сommit
0bcf5eee39
4 измененных файлов с 9 добавлено и 5 удалено
  1. 3 1
      include/corosync/cmap.h
  2. 3 1
      lib/cmap.c
  3. 1 1
      lib/sam.c
  4. 2 2
      tools/corosync-quorumtool.c

+ 3 - 1
include/corosync/cmap.h

@@ -301,7 +301,9 @@ extern cs_error_t cmap_iter_init(cmap_handle_t handle, const char *prefix, cmap_
  *
  * @param handle cmap handle
  * @param iter_handle handle of iteration returned by cmap_iter_init
- * @param key_name place to store name of key. Maximum length is CMAP_KEYNAME_MAXLEN
+ * @param key_name place to store name of key. Maximum length is CMAP_KEYNAME_MAXLEN and
+ *                 trailing zero is always added so size of the buffer has to be at least
+ *                 CMAP_KEYNAME_MAXLEN + 1.
  * @param value_len length of value
  * @param type type of value
  * @return CS_NO_SECTION if there are no more sections to iterate

+ 3 - 1
lib/cmap.c

@@ -875,7 +875,9 @@ cs_error_t cmap_iter_next(
 	}
 
 	if (error == CS_OK) {
-		strncpy(key_name, (const char *)res_lib_cmap_iter_next.key_name.value, CMAP_KEYNAME_MAXLEN);
+		memcpy(key_name, (const char *)res_lib_cmap_iter_next.key_name.value,
+		    res_lib_cmap_iter_next.key_name.length);
+		key_name[res_lib_cmap_iter_next.key_name.length] = '\0';
 
 		if (value_len != NULL) {
 			*value_len = res_lib_cmap_iter_next.value_len;

+ 1 - 1
lib/sam.c

@@ -216,7 +216,7 @@ static cs_error_t sam_cmap_destroy_pid_path (void)
 {
 	cmap_iter_handle_t iter;
 	cs_error_t err;
-	char key_name[CMAP_KEYNAME_MAXLEN];
+	char key_name[CMAP_KEYNAME_MAXLEN + 1];
 
 	err = cmap_iter_init(sam_internal_data.cmap_handle, sam_internal_data.cmap_pid_path, &iter);
 	if (err != CS_OK) {

+ 2 - 2
tools/corosync-quorumtool.c

@@ -259,8 +259,8 @@ static int set_expected(int expected_votes)
 static const char *node_name_by_nodelist(uint32_t nodeid)
 {
 	cmap_iter_handle_t iter;
-	char key_name[CMAP_KEYNAME_MAXLEN];
-	char tmp_key[CMAP_KEYNAME_MAXLEN];
+	char key_name[CMAP_KEYNAME_MAXLEN + 1];
+	char tmp_key[CMAP_KEYNAME_MAXLEN + 1];
 	static char ret_buf[_POSIX_HOST_NAME_MAX];
 	char *str = NULL;
 	uint32_t node_pos, cur_nodeid;