On Sat, Apr 01, 2023 at 09:08:49PM +0200, Otto Moerbeek wrote:
> Hi,
> 
> by default an allocation isn't fully written with junk bytes, only at
> certain spots. This introduces variations in the spot, so we have a
> bigger chance of catching write-after-frees in specific spots.
> 
> After a remark from jsing@ that variation would be nice to have for
> this.

This works well in my testing and I think this helps quite a bit. Adding
extra complications to avoid the modulo bias doesn't seem worth the effort.

ok tb

> 
>       -Otto
> 
> Index: stdlib/malloc.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.279
> diff -u -p -r1.279 malloc.c
> --- stdlib/malloc.c   1 Apr 2023 18:47:51 -0000       1.279
> +++ stdlib/malloc.c   1 Apr 2023 19:06:56 -0000
> @@ -221,6 +221,7 @@ struct malloc_readonly {
>       u_int   chunk_canaries;         /* use canaries after chunks? */
>       int     internal_funcs;         /* use better recallocarray/freezero? */
>       u_int   def_maxcache;           /* free pages we cache */
> +     u_int   junk_loc;               /* variation in location of junk */
>       size_t  malloc_guard;           /* use guard pages after allocations? */
>  #ifdef MALLOC_STATS
>       int     malloc_stats;           /* dump statistics at end */
> @@ -493,6 +494,7 @@ omalloc_init(void)
>  
>       while ((mopts.malloc_canary = arc4random()) == 0)
>               ;
> +     mopts.junk_loc = arc4random();
>       if (mopts.chunk_canaries)
>               do {
>                       mopts.chunk_canaries = arc4random();
> @@ -676,7 +678,9 @@ junk_free(int junk, void *p, size_t sz)
>               if (step == 0)
>                       step = 1;
>       }
> -     for (i = 0; i < sz; i += step)
> +     /* Do not always put the free junk bytes in the same spot.
> +        There is modulo bias here, but we ignore that. */
> +     for (i = mopts.junk_loc % step; i < sz; i += step)
>               lp[i] = SOME_FREEJUNK_ULL;
>  }
>  
> @@ -696,7 +700,8 @@ validate_junk(struct dir_info *pool, voi
>               if (step == 0)
>                       step = 1;
>       }
> -     for (i = 0; i < sz; i += step) {
> +     /* see junk_free */
> +     for (i = mopts.junk_loc % step; i < sz; i += step) {
>               if (lp[i] != SOME_FREEJUNK_ULL)
>                       wrterror(pool, "write after free %p", p);
>       }
> 
> 

Reply via email to