[v2,5/7] malloc: Verify the integrity of mmapped chunks in calloc.

Message ID 1510068430-27816-6-git-send-email-pistukem@gmail.com
State New, archived
Headers

Commit Message

Istvan Kurucsai Nov. 7, 2017, 3:27 p.m. UTC
  * malloc/malloc.c (__libc_calloc): Check mmapped chunks.
---
 malloc/malloc.c | 9 +++++++++
 1 file changed, 9 insertions(+)
  

Comments

Florian Weimer Aug. 17, 2018, 2:14 p.m. UTC | #1
On 11/07/2017 04:27 PM, Istvan Kurucsai wrote:
>      * malloc/malloc.c (__libc_calloc): Check mmapped chunks.
> ---
>   malloc/malloc.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 8e48952..5eb661e 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3447,6 +3447,15 @@ __libc_calloc (size_t n, size_t elem_size)
>     /* Two optional cases in which clearing not necessary */
>     if (chunk_is_mmapped (p))
>       {
> +      size_t pagesize = GLRO (dl_pagesize);
> +      INTERNAL_SIZE_T offset = prev_size (p);
> +      INTERNAL_SIZE_T size = chunksize (p);
> +      uintptr_t block = (uintptr_t) p - offset;
> +      size_t total_size = offset + size;
> +      if (__glibc_unlikely ((block | total_size) & (pagesize - 1)) != 0
> +          || __glibc_unlikely (!powerof2 ((uintptr_t) mem & (pagesize - 1))))
> +        malloc_printerr ("calloc(): invalid mmapped chunk");
> +
>         if (__builtin_expect (perturb_byte, 0))
>           return memset (mem, 0, sz);

Sorry, I don't understand the powerof2 check.  Could you elaborate?

Thanks,
Florian
  
Florian Weimer Nov. 16, 2018, 10:33 a.m. UTC | #2
* Florian Weimer:

> On 11/07/2017 04:27 PM, Istvan Kurucsai wrote:
>>      * malloc/malloc.c (__libc_calloc): Check mmapped chunks.
>> ---
>>   malloc/malloc.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index 8e48952..5eb661e 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
>> @@ -3447,6 +3447,15 @@ __libc_calloc (size_t n, size_t elem_size)
>>     /* Two optional cases in which clearing not necessary */
>>     if (chunk_is_mmapped (p))
>>       {
>> +      size_t pagesize = GLRO (dl_pagesize);
>> +      INTERNAL_SIZE_T offset = prev_size (p);
>> +      INTERNAL_SIZE_T size = chunksize (p);
>> +      uintptr_t block = (uintptr_t) p - offset;
>> +      size_t total_size = offset + size;
>> +      if (__glibc_unlikely ((block | total_size) & (pagesize - 1)) != 0
>> +          || __glibc_unlikely (!powerof2 ((uintptr_t) mem & (pagesize - 1))))
>> +        malloc_printerr ("calloc(): invalid mmapped chunk");
>> +
>>         if (__builtin_expect (perturb_byte, 0))
>>           return memset (mem, 0, sz);
>
> Sorry, I don't understand the powerof2 check.  Could you elaborate?

I believe this is similar to the mremap_chunk case DJ reviewed.

Thanks,
Florian
  

Patch

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 8e48952..5eb661e 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3447,6 +3447,15 @@  __libc_calloc (size_t n, size_t elem_size)
   /* Two optional cases in which clearing not necessary */
   if (chunk_is_mmapped (p))
     {
+      size_t pagesize = GLRO (dl_pagesize);
+      INTERNAL_SIZE_T offset = prev_size (p);
+      INTERNAL_SIZE_T size = chunksize (p);
+      uintptr_t block = (uintptr_t) p - offset;
+      size_t total_size = offset + size;
+      if (__glibc_unlikely ((block | total_size) & (pagesize - 1)) != 0
+          || __glibc_unlikely (!powerof2 ((uintptr_t) mem & (pagesize - 1))))
+        malloc_printerr ("calloc(): invalid mmapped chunk");
+
       if (__builtin_expect (perturb_byte, 0))
         return memset (mem, 0, sz);