On Fri, Sep 16, 2016 at 09:30:15PM +0200, Otto Moerbeek wrote:
> On Thu, Sep 15, 2016 at 10:08:26AM -0400, Ted Unangst wrote:
>
> > Otto Moerbeek wrote:
> > > On Wed, Sep 14, 2016 at 12:53:05PM -0400, Ted Unangst wrote:
> > >
> > > > Daniel Micay wrote:
> > > > >
> > > > > The current OpenBSD code only wipes up to MALLOC_MAXCHUNK with junk @
> > > > > 1,
> > > > > and it similarly doesn't wipe at all with 'U' (even though
> > > > > junk-on-free
> > > > > also serves the purpose of preventing information leaks, not just
> > > > > mitigating use-after-free). IMO, optimizing large allocation perf like
> > > > > this isn't worthwhile.
> > > >
> > > > this requires some analysis of what programs do in the wild. some
> > > > programs
> > > > preemptively malloc large buffers, but don't touch them. it would be a
> > > > serious
> > > > reqression for free to fault in new pages, just to ditry them, then turn
> > > > around and unmap them. some of this is because i believe the code is
> > > > doing
> > > > things at the wrong time. if you want to dirty whole pages, it should
> > > > be when
> > > > they go on the freelist, not immediately.
> > >
> > > Something like this?
> >
> > didn't look too closely, but looks good from a distance. :)
> >
>
> Sligtly better diff: it gets rid of a PAGEROUND(sz) call, since sz ==
> PAGEROUND(sz) in unmap().
Actually, I think the - mopts.malloc_guard can go as well. We're
dealing with unguarded regions at this spot.
>
> -Otto
>
> Index: malloc.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.195
> diff -u -p -r1.195 malloc.c
> --- malloc.c 1 Sep 2016 10:41:02 -0000 1.195
> +++ malloc.c 16 Sep 2016 19:26:34 -0000
> @@ -376,6 +376,11 @@ unmap(struct dir_info *d, void *p, size_
> for (i = 0; i < mopts.malloc_cache; i++) {
> r = &d->free_regions[(i + offset) & (mopts.malloc_cache - 1)];
> if (r->p == NULL) {
> + if (mopts.malloc_junk && !mopts.malloc_freeunmap) {
> + size_t amt = mopts.malloc_junk == 1 ?
> + MALLOC_MAXCHUNK : sz - mopts.malloc_guard;
> + memset(p, SOME_FREEJUNK, amt);
> + }
> if (mopts.malloc_hint)
> madvise(p, sz, MADV_FREE);
> if (mopts.malloc_freeunmap)
> @@ -1335,11 +1340,6 @@ ofree(struct dir_info *argpool, void *p)
> wrterror(pool, "mprotect", NULL);
> }
> STATS_SUB(pool->malloc_guarded, mopts.malloc_guard);
> - }
> - if (mopts.malloc_junk && !mopts.malloc_freeunmap) {
> - size_t amt = mopts.malloc_junk == 1 ? MALLOC_MAXCHUNK :
> - PAGEROUND(sz) - mopts.malloc_guard;
> - memset(p, SOME_FREEJUNK, amt);
> }
> unmap(pool, p, PAGEROUND(sz));
> delete(pool, r);
