[Swan-dev] pfree checks

[Paul Wouters]

Seems useful to me

Sent from my iPhone

> On Oct 7, 2017, at 13:29, Andrew Cagney <andrew.cagney at gmail.com> wrote:
> 
> I'm considering a commit containing some of the below additional checks:
> 
> - try to spot a double free (I think this check should go in)
> 
> - try to spot a use-after-free, not so sure about this
> 
> - and there is also the possibility of a check for heap overflow
> 
> but these start to stray into checks that free() is doing anyway :-/
> 
> thoughts,
> Andrew
> 
> diff --git a/lib/libswan/alloc.c b/lib/libswan/alloc.c
> index ab6f3194b..c1f9824e7 100644
> --- a/lib/libswan/alloc.c
> +++ b/lib/libswan/alloc.c
> @@ -61,6 +61,20 @@ void set_alloc_exit_log_func(exit_log_func_t func)
>  /* this magic number is 3671129837 decimal (623837458 complemented) */
>  #define LEAK_MAGIC 0xDAD0FEEDul
>  
> +#define INVALID_MAGIC 0xFE
> +#if __SIZEOF_POINTER__ == 4
> +#define INVALID_POINTER ((void*)0xFEFEFEFEul)
> +#endif
> +#if __SIZEOF_POINTER__ == 8
> +#define INVALID_POINTER ((void*)0xFEFEFEFEFEFEFEFEull)
> +#endif
> +#ifndef INVALID_POINTER
> +/* suppresses check */
> +#define INVALID_POINTER NULL
> +#endif
> +
> +static bool check_invalid_pointer = true;
> +
>  union mhdr {
>      struct {
>          const char *name;
> @@ -116,6 +130,9 @@ static void *alloc_bytes_raw(size_t size, const char *name)
>              allocs = p;
>              pthread_mutex_unlock(&leak_detective_mutex);
>          }
> +        if (p + 1 == INVALID_POINTER) {
> +            check_invalid_pointer = false; /* sigh! */
> +        }
>          return p + 1;
>      } else {
>          return p;
> @@ -128,8 +145,20 @@ void pfree(void *ptr)
>          union mhdr *p;
>  
>          passert(ptr != NULL);
> +        if (check_invalid_pointer) {
> +            if (ptr == INVALID_POINTER) {
> +                PASSERT_FAIL("pointer %p invalid (likely from use after free)", p);
> +            }
> +        }
> +
>          p = ((union mhdr *)ptr) - 1;
> -        passert(p->i.magic == LEAK_MAGIC);
> +
> +        if (p->i.magic == ~LEAK_MAGIC) {
> +            PASSERT_FAIL("pointer %p invalid (likely from double free)", ptr);
> +        } else if (p->i.magic != LEAK_MAGIC) {
> +            PASSERT_FAIL("pointer %p invalid (likely from heap corruption)", ptr);
> +        }
> +
>          {
>              pthread_mutex_lock(&leak_detective_mutex);
>              if (p->i.older != NULL) {
> @@ -146,7 +175,7 @@ void pfree(void *ptr)
>              pthread_mutex_unlock(&leak_detective_mutex);
>          }
>          /* stomp on memory!   Is another byte value better? */
> -        memset(p, 0xEF, sizeof(union mhdr) + p->i.size);
> +        memset(p, INVALID_MAGIC, sizeof(union mhdr) + p->i.size);
>          p->i.magic = ~LEAK_MAGIC;
>          free(p);
>      } else {
> 
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev