On 22.01.2016 16:02, Kenneth Graunke wrote:
On Friday, January 22, 2016 12:09:18 PM PST Nicolai Hähnle wrote:
On 22.01.2016 02:53, Jordan Justen wrote:
Juha-Pekka found this back in May 2015:
<1430915727-28677-1-git-send-email-juhapekka.heikk...@gmail.com>
From the discussion, obviously it would be preferable to make
ralloc_size no longer return zeroed memory, but Juha-Pekka found that
it would break Mesa.
For now, let's point out the flaw, and stop doing the double zeroing
of rzalloc buffers.
Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikk...@gmail.com>
Cc: Kenneth Graunke <kenn...@whitecape.org>
---
For a release build, I saw the code size shrink by 64 bytes.
src/util/ralloc.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/util/ralloc.c b/src/util/ralloc.c
index 6d4032b..24c1eee 100644
--- a/src/util/ralloc.c
+++ b/src/util/ralloc.c
@@ -49,6 +49,14 @@ _CRTIMP int _vscprintf(const char *format, va_list
argptr);
#endif
#endif
+/* ralloc_size has always used calloc to allocate memory. This has
allowed
+ * code using ralloc_size to depend on always receiving a cleared buffer.
+ *
+ * FIXME: Clean up the code base to allow this to be set to false, and
then
+ * remove it altogether.
+ */
+static const bool always_allocate_zeroed_memory = true;
+
#define CANARY 0x5A1106
struct ralloc_header
@@ -110,7 +118,10 @@ ralloc_context(const void *ctx)
void *
ralloc_size(const void *ctx, size_t size)
{
- void *block = calloc(1, size + sizeof(ralloc_header));
+ void *block =
+ always_allocate_zeroed_memory ?
+ calloc(1, size + sizeof(ralloc_header)) :
+ malloc(size + sizeof(ralloc_header));
There's an integer overflow here which would be good to fix.
Please explain? ralloc_header is 40-44 bytes - the only way this will
overflow is if you asked for an absurd amount of memory (already near
the max value of size_t). And, if you did, I'm not sure what we're
supposed to do about it...
A common method of triggering buffer overflows leading to security
exploits is that the attacker sets an absurdly large buffer size
somewhere - so large that additional calculations that increase the
buffer size wrap around and result in a very small successful
allocation. The code will then write memory based on the original,
absurdly large buffer size. This means writing beyond the end of the
allocated buffer.
The people who look for security exploits for a living are really good
at finding ways in which such a situation can then be used to hijack the
control flow of the program somehow (for example, targeted overwriting
of the internal metadata of the heap... sounds crazy, but it's used) to
do more or less whatever they want.
I think a decent way to protect against it is something like:
/* Overflow of unsigned integer types is well defined */
if (size + sizeof(ralloc_header) < size)
return NULL;
or perhaps
if (size > SIZE_MAX - sizeof(ralloc_header))
return NULL;
Newer versions of gcc have nicer builtins for arithmetic with overflow
check, but I don't know if we want to depend on those being available.
The hope is that the calling code properly handles allocation failure.
Even if it doesn't, the result is much more likely to just be a segfault
before anything dangerous can happen.
One may say that all of this depends on an attacker gaining access to
Mesa, but WebGL is a thing, so...
Cheers,
Nicolai
Since it was there already in the older version, the patch is
Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
as is.
ralloc_header *info;
ralloc_header *parent;
@@ -132,7 +143,7 @@ void *
rzalloc_size(const void *ctx, size_t size)
{
void *ptr = ralloc_size(ctx, size);
- if (likely(ptr != NULL))
+ if (!always_allocate_zeroed_memory && likely(ptr != NULL))
memset(ptr, 0, size);
return ptr;
}
Dropping the memset seems reasonable. I would prefer it if we simply
moved the contents of ralloc_size into rzalloc_size, and made
ralloc_size call rzalloc_size with a comment.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
