Avoid calling Standard C string/array functions with NULL arguments

glibc string.h declares memcpy() with attribute(nonnull(1,2)), causing
calls with NULL arguments to be treated as undefined behaviour.
This is consistent with ISO C99 and C11, which state that passing 0
to string functions as an array length does not remove the requirement
that the pointer to the array is a valid pointer.
gcc -fsanitize=undefined catches this while running OSTree's test suite.

Similarly, running the GLib test suite reports similar issues for
qsort(), memmove(), memcmp().

Signed-off-by: Simon McVittie <smcv@debian.org>
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=775510
Reviewed-by: Colin Walters
This commit is contained in:
Simon McVittie 2016-12-02 10:03:16 +00:00
parent 5e7eaaaaee
commit e5ed410c8c
10 changed files with 43 additions and 18 deletions

View File

@ -262,7 +262,9 @@ buffer_read (Buffer *buffer,
char *dest, char *dest,
gsize count) gsize count)
{ {
memcpy (dest, buffer->data + buffer->start, count); if (count != 0)
memcpy (dest, buffer->data + buffer->start, count);
buffer_consumed (buffer, count); buffer_consumed (buffer, count);
} }
@ -293,9 +295,11 @@ grow_buffer (Buffer *buffer)
data = g_malloc (size); data = g_malloc (size);
in_buffer = buffer_data_size (buffer); in_buffer = buffer_data_size (buffer);
memcpy (data, if (in_buffer != 0)
buffer->data + buffer->start, memcpy (data,
in_buffer); buffer->data + buffer->start,
in_buffer);
g_free (buffer->data); g_free (buffer->data);
buffer->data = data; buffer->data = data;
buffer->end -= buffer->start; buffer->end -= buffer->start;

View File

@ -300,9 +300,11 @@ grow_buffer (Buffer *buffer)
data = g_malloc (size); data = g_malloc (size);
in_buffer = buffer_data_size (buffer); in_buffer = buffer_data_size (buffer);
memcpy (data, if (in_buffer != 0)
buffer->data + buffer->start, memcpy (data,
in_buffer); buffer->data + buffer->start,
in_buffer);
g_free (buffer->data); g_free (buffer->data);
buffer->data = data; buffer->data = data;
buffer->end -= buffer->start; buffer->end -= buffer->start;

View File

@ -506,7 +506,8 @@ add_token_result (const gchar *app_name,
static void static void
merge_token_results (gboolean first) merge_token_results (gboolean first)
{ {
qsort (static_token_results, static_token_results_size, sizeof (struct search_result), compare_results); if (static_token_results_size != 0)
qsort (static_token_results, static_token_results_size, sizeof (struct search_result), compare_results);
/* If this is the first token then we are basically merging a list with /* If this is the first token then we are basically merging a list with
* itself -- we only perform de-duplication. * itself -- we only perform de-duplication.
@ -606,7 +607,8 @@ reset_total_search_results (void)
static void static void
sort_total_search_results (void) sort_total_search_results (void)
{ {
qsort (static_total_results, static_total_results_size, sizeof (struct search_result), compare_categories); if (static_total_results_size != 0)
qsort (static_total_results, static_total_results_size, sizeof (struct search_result), compare_categories);
} }
static void static void
@ -620,9 +622,10 @@ merge_directory_results (void)
static_total_results = g_renew (struct search_result, static_total_results, static_total_results_allocated); static_total_results = g_renew (struct search_result, static_total_results, static_total_results_allocated);
} }
memcpy (static_total_results + static_total_results_size, if (static_total_results + static_total_results_size != 0)
static_search_results, memcpy (static_total_results + static_total_results_size,
static_search_results_size * sizeof (struct search_result)); static_search_results,
static_search_results_size * sizeof (struct search_result));
static_total_results_size += static_search_results_size; static_total_results_size += static_search_results_size;

View File

@ -116,7 +116,9 @@ g_unix_socket_address_set_property (GObject *object,
/* Clip to fit in UNIX_PATH_MAX with zero termination or first byte */ /* Clip to fit in UNIX_PATH_MAX with zero termination or first byte */
len = MIN (array->len, UNIX_PATH_MAX-1); len = MIN (array->len, UNIX_PATH_MAX-1);
memcpy (address->priv->path, array->data, len); if (len != 0)
memcpy (address->priv->path, array->data, len);
address->priv->path[len] = 0; /* Ensure null-terminated */ address->priv->path[len] = 0; /* Ensure null-terminated */
address->priv->path_len = len; address->priv->path_len = len;
} }

View File

@ -291,7 +291,8 @@ file_builder_add_string (FileBuilder *fb,
chunk->offset = fb->offset; chunk->offset = fb->offset;
chunk->size = length; chunk->size = length;
chunk->data = g_malloc (length); chunk->data = g_malloc (length);
memcpy (chunk->data, string, length); if (length != 0)
memcpy (chunk->data, string, length);
*start = guint32_to_le (fb->offset); *start = guint32_to_le (fb->offset);
*size = guint16_to_le (length); *size = guint16_to_le (length);

View File

@ -415,6 +415,9 @@ g_array_append_vals (GArray *farray,
g_return_val_if_fail (array, NULL); g_return_val_if_fail (array, NULL);
if (len == 0)
return farray;
g_array_maybe_expand (array, len); g_array_maybe_expand (array, len);
memcpy (g_array_elt_pos (array, array->len), data, memcpy (g_array_elt_pos (array, array->len), data,
@ -468,6 +471,9 @@ g_array_prepend_vals (GArray *farray,
g_return_val_if_fail (array, NULL); g_return_val_if_fail (array, NULL);
if (len == 0)
return farray;
g_array_maybe_expand (array, len); g_array_maybe_expand (array, len);
memmove (g_array_elt_pos (array, len), g_array_elt_pos (array, 0), memmove (g_array_elt_pos (array, len), g_array_elt_pos (array, 0),
@ -517,6 +523,9 @@ g_array_insert_vals (GArray *farray,
g_return_val_if_fail (array, NULL); g_return_val_if_fail (array, NULL);
if (len == 0)
return farray;
g_array_maybe_expand (array, len); g_array_maybe_expand (array, len);
memmove (g_array_elt_pos (array, len + index_), memmove (g_array_elt_pos (array, len + index_),

View File

@ -2361,7 +2361,10 @@ g_option_group_add_entries (GOptionGroup *group,
group->entries = g_renew (GOptionEntry, group->entries, group->n_entries + n_entries); group->entries = g_renew (GOptionEntry, group->entries, group->n_entries + n_entries);
memcpy (group->entries + group->n_entries, entries, sizeof (GOptionEntry) * n_entries); /* group->entries could be NULL in the trivial case where we add no
* entries to no entries */
if (n_entries != 0)
memcpy (group->entries + group->n_entries, entries, sizeof (GOptionEntry) * n_entries);
for (i = group->n_entries; i < group->n_entries + n_entries; i++) for (i = group->n_entries; i < group->n_entries + n_entries; i++)
{ {

View File

@ -386,7 +386,7 @@ g_memdup (gconstpointer mem,
{ {
gpointer new_mem; gpointer new_mem;
if (mem) if (mem && byte_size != 0)
{ {
new_mem = g_malloc (byte_size); new_mem = g_malloc (byte_size);
memcpy (new_mem, mem, byte_size); memcpy (new_mem, mem, byte_size);

View File

@ -74,7 +74,7 @@ typedef void (*GTestFixtureFunc) (gpointer fixture,
if (__l1 != __l2) \ if (__l1 != __l2) \
g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ g_assertion_message_cmpnum (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \
#l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==", __l2, 'i'); \ #l1 " (len(" #m1 ")) == " #l2 " (len(" #m2 "))", __l1, "==", __l2, 'i'); \
else if (memcmp (__m1, __m2, __l1) != 0) \ else if (__l1 != 0 && memcmp (__m1, __m2, __l1) != 0) \
g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \
"assertion failed (" #m1 " == " #m2 ")"); \ "assertion failed (" #m1 " == " #m2 ")"); \
} G_STMT_END } G_STMT_END

View File

@ -3550,7 +3550,8 @@ g_type_children (GType type,
G_READ_LOCK (&type_rw_lock); /* ->children is relocatable */ G_READ_LOCK (&type_rw_lock); /* ->children is relocatable */
children = g_new (GType, node->n_children + 1); children = g_new (GType, node->n_children + 1);
memcpy (children, node->children, sizeof (GType) * node->n_children); if (node->n_children != 0)
memcpy (children, node->children, sizeof (GType) * node->n_children);
children[node->n_children] = 0; children[node->n_children] = 0;
if (n_children) if (n_children)