From 5fcd2495f98c8e68a9a4b7fec1134df1fb2d8bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 25 Nov 2021 14:05:42 +0200 Subject: [PATCH 1/4] De-duplicate g_nearest_pow() implementation And put it as static inline function into a private shared header instead. --- gio/gdbusmessage.c | 12 +----------- gio/gmemoryoutputstream.c | 12 +----------- glib/garray.c | 24 +----------------------- glib/gstring.c | 24 ++---------------------- glib/gstringchunk.c | 30 +++++------------------------- glib/gutilsprivate.h | 24 ++++++++++++++++++++++++ 6 files changed, 34 insertions(+), 92 deletions(-) diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c index 0b803bc30..cc8f37e5d 100644 --- a/gio/gdbusmessage.c +++ b/gio/gdbusmessage.c @@ -51,6 +51,7 @@ #include "gseekable.h" #include "gioerror.h" #include "gdbusprivate.h" +#include "gutilsprivate.h" #ifdef G_OS_UNIX #include "gunixfdlist.h" @@ -257,17 +258,6 @@ g_memory_buffer_read_uint64 (GMemoryBuffer *mbuf, #define MIN_ARRAY_SIZE 128 -static gsize -g_nearest_pow (gsize num) -{ - gsize n = 1; - - while (n < num && n > 0) - n <<= 1; - - return n; -} - static void array_resize (GMemoryBuffer *mbuf, gsize size) diff --git a/gio/gmemoryoutputstream.c b/gio/gmemoryoutputstream.c index b120ccedc..6a410ebb5 100644 --- a/gio/gmemoryoutputstream.c +++ b/gio/gmemoryoutputstream.c @@ -29,6 +29,7 @@ #include "gioerror.h" #include "string.h" #include "glibintl.h" +#include "gutilsprivate.h" /** @@ -596,17 +597,6 @@ array_resize (GMemoryOutputStream *ostream, return TRUE; } -static gsize -g_nearest_pow (gsize num) -{ - gsize n = 1; - - while (n < num && n > 0) - n <<= 1; - - return n; -} - static gssize g_memory_output_stream_write (GOutputStream *stream, const void *buffer, diff --git a/glib/garray.c b/glib/garray.c index 254921247..e493e2ac1 100644 --- a/glib/garray.c +++ b/glib/garray.c @@ -42,6 +42,7 @@ #include "gmessages.h" #include "gqsort.h" #include "grefcount.h" +#include "gutilsprivate.h" /** * SECTION:arrays @@ -159,7 +160,6 @@ struct _GRealArray g_array_elt_zero ((array), (array)->len, 1); \ }G_STMT_END -static gsize g_nearest_pow (gsize num) G_GNUC_CONST; static void g_array_maybe_expand (GRealArray *array, guint len); @@ -972,28 +972,6 @@ g_array_binary_search (GArray *array, return result; } -/* Returns the smallest power of 2 greater than or equal to n, - * or 0 if such power does not fit in a gsize - */ -static gsize -g_nearest_pow (gsize num) -{ - gsize n = num - 1; - - g_assert (num > 0 && num <= G_MAXSIZE / 2); - - n |= n >> 1; - n |= n >> 2; - n |= n >> 4; - n |= n >> 8; - n |= n >> 16; -#if GLIB_SIZEOF_SIZE_T == 8 - n |= n >> 32; -#endif - - return n + 1; -} - static void g_array_maybe_expand (GRealArray *array, guint len) diff --git a/glib/gstring.c b/glib/gstring.c index 17dc2c33d..05b20b3e3 100644 --- a/glib/gstring.c +++ b/glib/gstring.c @@ -37,6 +37,7 @@ #include "gstring.h" #include "guriprivate.h" #include "gprintf.h" +#include "gutilsprivate.h" /** @@ -71,34 +72,13 @@ * The GString struct contains the public fields of a GString. */ - -#define MY_MAXSIZE ((gsize)-1) - -static inline gsize -nearest_power (gsize base, gsize num) -{ - if (num > MY_MAXSIZE / 2) - { - return MY_MAXSIZE; - } - else - { - gsize n = base; - - while (n < num) - n <<= 1; - - return n; - } -} - static void g_string_maybe_expand (GString *string, gsize len) { if (string->len + len >= string->allocated_len) { - string->allocated_len = nearest_power (1, string->len + len + 1); + string->allocated_len = g_nearest_pow (string->len + len + 1); string->str = g_realloc (string->str, string->allocated_len); } } diff --git a/glib/gstringchunk.c b/glib/gstringchunk.c index 524eadcef..226bfa98f 100644 --- a/glib/gstringchunk.c +++ b/glib/gstringchunk.c @@ -37,6 +37,7 @@ #include "gmessages.h" #include "gutils.h" +#include "gutilsprivate.h" /** * SECTION:string_chunks @@ -82,27 +83,6 @@ struct _GStringChunk gsize default_size; }; -#define MY_MAXSIZE ((gsize)-1) - -static inline gsize -nearest_power (gsize base, - gsize num) -{ - if (num > MY_MAXSIZE / 2) - { - return MY_MAXSIZE; - } - else - { - gsize n = base; - - while (n < num) - n <<= 1; - - return n; - } -} - /** * g_string_chunk_new: * @size: the default size of the blocks of memory which are @@ -120,7 +100,7 @@ g_string_chunk_new (gsize size) GStringChunk *new_chunk = g_new (GStringChunk, 1); gsize actual_size = 1; - actual_size = nearest_power (1, size); + actual_size = g_nearest_pow (MAX (1, size)); new_chunk->const_table = NULL; new_chunk->storage_list = NULL; @@ -280,7 +260,7 @@ g_string_chunk_insert_len (GStringChunk *chunk, const gchar *string, gssize len) { - gssize size; + gsize size; gchar* pos; g_return_val_if_fail (chunk != NULL, NULL); @@ -288,11 +268,11 @@ g_string_chunk_insert_len (GStringChunk *chunk, if (len < 0) size = strlen (string); else - size = len; + size = (gsize) len; if ((chunk->storage_next + size + 1) > chunk->this_size) { - gsize new_size = nearest_power (chunk->default_size, size + 1); + gsize new_size = g_nearest_pow (MAX (chunk->default_size, size + 1)); chunk->storage_list = g_slist_prepend (chunk->storage_list, g_new (gchar, new_size)); diff --git a/glib/gutilsprivate.h b/glib/gutilsprivate.h index 5a0686086..a3996f8b7 100644 --- a/glib/gutilsprivate.h +++ b/glib/gutilsprivate.h @@ -20,7 +20,9 @@ #ifndef __G_UTILS_PRIVATE_H__ #define __G_UTILS_PRIVATE_H__ +#include "glibconfig.h" #include "gtypes.h" +#include "gtestutils.h" G_BEGIN_DECLS @@ -28,6 +30,28 @@ GLIB_AVAILABLE_IN_2_60 void g_set_user_dirs (const gchar *first_dir_type, ...) G_GNUC_NULL_TERMINATED; +/* Returns the smallest power of 2 greater than or equal to n, + * or 0 if such power does not fit in a gsize + */ +static inline gsize +g_nearest_pow (gsize num) +{ + gsize n = num - 1; + + g_assert (num > 0 && num <= G_MAXSIZE / 2); + + n |= n >> 1; + n |= n >> 2; + n |= n >> 4; + n |= n >> 8; + n |= n >> 16; +#if GLIB_SIZEOF_SIZE_T == 8 + n |= n >> 32; +#endif + + return n + 1; +} + G_END_DECLS #endif /* __G_UTILS_PRIVATE_H__ */ From d01dc6d23a686778d8c0f1df695a3957f363f656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 25 Nov 2021 14:11:29 +0200 Subject: [PATCH 2/4] Add stricter overflow protection from GArray to g_ptr_array_maybe_expand() too It might otherwise happen that the return value from g_nearest_pow() does not fit into a guint, i.e. it might be G_MAXUINT + 1 if that fits into a gsize. --- glib/garray.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/glib/garray.c b/glib/garray.c index e493e2ac1..3803fee03 100644 --- a/glib/garray.c +++ b/glib/garray.c @@ -1503,8 +1503,16 @@ static void g_ptr_array_maybe_expand (GRealPtrArray *array, guint len) { + guint max_len; + + /* The maximum array length is derived from following constraints: + * - The number of bytes must fit into a gsize / 2. + * - The number of elements must fit into guint. + */ + max_len = MIN (G_MAXSIZE / 2 / sizeof (gpointer), G_MAXUINT); + /* Detect potential overflow */ - if G_UNLIKELY ((G_MAXUINT - array->len) < len) + if G_UNLIKELY ((max_len - array->len) < len) g_error ("adding %u to array would overflow", len); if ((array->len + len) > array->alloc) From b5447e8e35e42e77539c21710fc26979cf096846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 25 Nov 2021 14:19:53 +0200 Subject: [PATCH 3/4] Add overflow protection to g_string_maybe_expand() --- glib/gstring.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/glib/gstring.c b/glib/gstring.c index 05b20b3e3..0a509e5e5 100644 --- a/glib/gstring.c +++ b/glib/gstring.c @@ -76,9 +76,17 @@ static void g_string_maybe_expand (GString *string, gsize len) { + /* Detect potential overflow */ + if G_UNLIKELY ((G_MAXSIZE - string->len - 1) < len) + g_error ("adding %" G_GSIZE_FORMAT " to string would overflow", len); + if (string->len + len >= string->allocated_len) { string->allocated_len = g_nearest_pow (string->len + len + 1); + /* If the new size is bigger than G_MAXSIZE / 2, only allocate enough + * memory for this string and don't over-allocate. */ + if (string->allocated_len == 0) + string->allocated_len = string->len + len + 1; string->str = g_realloc (string->str, string->allocated_len); } } From 72ca69e1dbf765de1b19fa0769cca614057a8d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 25 Nov 2021 14:25:24 +0200 Subject: [PATCH 4/4] Add some overflow protection to g_string_chunk_insert_len() If the new string's length plus the existing storage's length is overflowing a gsize, we would previously memcpy() the string over the bounds of the previous allocation. Similarly if the string's size was bigger than G_MAXSIZE / 2 we would've previously allocated 0 bytes. Now instead create a new allocation that fits the string. --- glib/gstringchunk.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/glib/gstringchunk.c b/glib/gstringchunk.c index 226bfa98f..feacb154f 100644 --- a/glib/gstringchunk.c +++ b/glib/gstringchunk.c @@ -270,10 +270,15 @@ g_string_chunk_insert_len (GStringChunk *chunk, else size = (gsize) len; - if ((chunk->storage_next + size + 1) > chunk->this_size) + if ((G_MAXSIZE - chunk->storage_next < size + 1) || (chunk->storage_next + size + 1) > chunk->this_size) { gsize new_size = g_nearest_pow (MAX (chunk->default_size, size + 1)); + /* If size is bigger than G_MAXSIZE / 2 then store it in its own + * allocation instead of failing here */ + if (new_size == 0) + new_size = size + 1; + chunk->storage_list = g_slist_prepend (chunk->storage_list, g_new (gchar, new_size));