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); > } > >