On Fri, Feb 24, 2023 at 08:13:13AM +0100, Otto Moerbeek wrote:
> On Sat, Feb 18, 2023 at 04:12:08PM +0100, Otto Moerbeek wrote:
>
> > Hi,
> >
> > these recent sshd double free issue prompted me to look at malloc
> > again. I have something bigger brewing, but this diff makes sure the
> > to be cleaned chunks (via freezero() or malloc_conceal) particpate in
> > the delayed freeing just as others.
>
> I'd like to see tests/reviews of this.
I have tested this for a few days on my main machines (amd64 and arm64).
I also tested it on sparc64 and i386 (machines mostly used for building).
The diff reads fine
ok tb
>
> >
> > -Otto
> >
> > Index: stdlib/malloc.c
> > ===================================================================
> > RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
> > retrieving revision 1.276
> > diff -u -p -r1.276 malloc.c
> > --- stdlib/malloc.c 27 Dec 2022 17:31:09 -0000 1.276
> > +++ stdlib/malloc.c 16 Feb 2023 14:11:50 -0000
> > @@ -1515,42 +1515,38 @@ ofree(struct dir_info **argpool, void *p
> > unmap(pool, p, PAGEROUND(sz), clear ? argsz : 0);
> > delete(pool, r);
> > } else {
> > + void *tmp;
> > + u_int i;
> > +
> > /* Validate and optionally canary check */
> > struct chunk_info *info = (struct chunk_info *)r->size;
> > if (info->size != sz)
> > wrterror(pool, "internal struct corrupt");
> > find_chunknum(pool, info, p, mopts.chunk_canaries);
> > - if (!clear) {
> > - void *tmp;
> > - int i;
> >
> > - if (mopts.malloc_freecheck) {
> > - for (i = 0; i <= MALLOC_DELAYED_CHUNK_MASK; i++)
> > - if (p == pool->delayed_chunks[i])
> > - wrterror(pool,
> > - "double free %p", p);
> > - }
> > - junk_free(pool->malloc_junk, p, sz);
> > - i = getrbyte(pool) & MALLOC_DELAYED_CHUNK_MASK;
> > - tmp = p;
> > - p = pool->delayed_chunks[i];
> > - if (tmp == p)
> > - wrterror(pool, "double free %p", tmp);
> > - pool->delayed_chunks[i] = tmp;
> > - if (p != NULL) {
> > - r = find(pool, p);
> > - REALSIZE(sz, r);
> > - if (r != NULL)
> > - validate_junk(pool, p, sz);
> > - }
> > - } else if (argsz > 0) {
> > - r = find(pool, p);
> > - explicit_bzero(p, argsz);
> > + if (mopts.malloc_freecheck) {
> > + for (i = 0; i <= MALLOC_DELAYED_CHUNK_MASK; i++)
> > + if (p == pool->delayed_chunks[i])
> > + wrterror(pool,
> > + "double free %p", p);
> > }
> > + if (clear && argsz > 0)
> > + explicit_bzero(p, argsz);
> > + junk_free(pool->malloc_junk, p, sz);
> > +
> > + i = getrbyte(pool) & MALLOC_DELAYED_CHUNK_MASK;
> > + tmp = p;
> > + p = pool->delayed_chunks[i];
> > + if (tmp == p)
> > + wrterror(pool, "double free %p", p);
> > + pool->delayed_chunks[i] = tmp;
> > if (p != NULL) {
> > + r = find(pool, p);
> > if (r == NULL)
> > wrterror(pool,
> > "bogus pointer (double free?) %p", p);
> > + REALSIZE(sz, r);
> > + validate_junk(pool, p, sz);
> > free_bytes(pool, r, p);
> > }
> > }
> >
>
