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