Project

General

Profile

Bug #10548

libumem doesn't free memory allocated by valloc or memalign

Added by Andy Fiddaman 2 months ago. Updated 2 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
2019-03-13
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

This is from Delphix DLPX-54735, work by Pavel Zakharov. Here is his analysis:

It appears that vmem_xfree() that is responsible of freeing segments of memory actually attempts to return the segment to the parent arena if it was imported from it.

In vmem_xalloc(), we import segments from the parent by calling vmp->vm_source_alloc() and then vmem_span_create() with import = 1, which so far makes sense.
Later on, we take the free segment created by vmem_span_create and call vmem_seg_alloc(), which allocates our memory with the needed alignment and size. However, if the span that we created is bigger than the segment we need, we'll need to split our span as follows:

|--------------------FREE---------------------|

                      |
                      V

|-FREE-|--------ALLOC---------|------FREE-----|

This is done in vmem_seg_alloc by calling vmem_seg_create for the 2 new free segments, and then resizing the original free segment and changing it's type to ALLOC with those 3 lines:

vsp->vs_start = addr;
vsp->vs_end = addr + size;

vmem_hash_insert(vmp, vsp);

This ALLOC segment will keep the same vs_import flag as the original segment, which is good, however the 2 new created FREE segments will have vs_import default to 0.

As a result, when vmem_xfree is later called on the ALLOC'd bloc, we will coalesce it with the two small free segments (provided they are still free), as per those checks:

if (vnext->vs_type == VMEM_FREE) { ... }

if (vprev->vs_type == VMEM_FREE) { ... }
vsp = vprev;

The catch here is that the second coalescing operation sets vsp = vprev and vprev had vs_import = 0 .

As a result, The next check will never pass and always execute the else clause:

/*
* If the entire span is free, return it to the source.
*/
if (vsp->vs_import && vmp->vm_source_free != NULL &&
    vsp->vs_aprev->vs_type == VMEM_SPAN &&
    vsp->vs_anext->vs_type == VMEM_SPAN) {
        vaddr = (void *)vsp->vs_start;
        size = VS_SIZE(vsp);
        ASSERT(size == VS_SIZE(vsp->vs_aprev));
        vmem_span_destroy(vmp, vsp);
        (void) mutex_unlock(&vmp->vm_lock);
        vmp->vm_source_free(vmp->vm_source, vaddr, size);
} else {
        vmem_freelist_insert(vmp, vsp);
        (void) mutex_unlock(&vmp->vm_lock);
}

which will put the segment on the freelist of the arena instead of returning it to the parent.

For the fix, I spoke with Jonathan Adams, who originally ported kmem into libumem. This error was found and fixed in kmem before the original OpenSolaris release, but never made it into libumem.

Note that this bug was observed when querying libdevinfo through JNA in our stack. We had a growing list of devices returned by libdevinfo, which meant that larger and larger memory regions were requested over time and the previously allocated smaller regions were not freed. If you always request the same memory chunk size, then the previous memory spans will be reused.

History

#1

Updated by Andy Fiddaman 2 months ago

Review at: https://illumos.org/rb/r/1564/

In particular, note:

This error was found and fixed in kmem before the original OpenSolaris release, but never made it into libumem.

You can verify this by reviewing the code in https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/os/vmem.c#L557

#2

Updated by Andy Fiddaman 2 months ago

Using this simple program that uses valign() to allocate and free increasing sizes of memory:

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>

int
main()
{
        size_t sz;

        for (sz = 998; sz < 100000000; sz += 998)
                free(valloc(sz));

        printf("Done allocating.\n");

        for (;;)
                sleep(3600);

        return 0;
}

Before the change:

% ./10548_umem &
[1] 8031
% pmap -x 8031
8031:   ./10548_umem
 Address  Kbytes     RSS    Anon  Locked Mode   Mapped File
08046000       8       8       8       - rwx--    [ stack ]
08050000       8       8       -       - r-x--  10548_umem
08061000       4       4       4       - rwx--  10548_umem
08E7B000 3839764    6056    6056       - rwx--    [ heap ]
F3670000       4       4       4       - rwx--    [ anon ]
FAAC0000       4       4       4       - rw---    [ anon ]
FAE40000       4       4       4       - rw---    [ anon ]
FAFB0000       4       4       4       - rwx--    [ anon ]
FCA80000     196     196       -       - r-x--  libumem.so.1
FCAC0000       8       8       8       - rwx--  libumem.so.1
FCAD2000      76      72      72       - rw---  libumem.so.1
FCAE5000      24      24      24       - rw---  libumem.so.1
FDCC0000       4       4       4       - rwx--    [ anon ]
FE820000    1300     960       -       - r-x--  libc_hwcap1.so.1
FE975000      40      40      36       - rwx--  libc_hwcap1.so.1
FE97F000      16       8       8       - rwx--  libc_hwcap1.so.1
FE9C0000      24      12      12       - rwx--    [ anon ]
FEFB0000       4       4       -       -r--s-
FEFB4000     220     220       -       - r-x--  ld.so.1
FEFFB000       8       8       8       - rwx--  ld.so.1
FEFFD000       4       4       4       - rwx--  ld.so.1
-------- ------- ------- ------- -------
total Kb 3841724    7652    6260       -

and after:

% LD_LIBRARY_PATH=/data/omnios-build/omniosorg/bloody/illumos/proto/root_i386-nd/usr/lib ./10548_umem &
[1] 8056
Done allocating.
% pmap -x 8056
8056:   ./10548_umem
 Address  Kbytes     RSS    Anon  Locked Mode   Mapped File
08041000      28      28      28       - rwx--    [ stack ]
08050000       8       8       -       - r-x--  10548_umem
08061000       4       4       4       - rwx--  10548_umem
0877D000  102684     292     292       - rwx--    [ heap ]
F3350000       4       4       4       - rwx--    [ anon ]
F9DB0000       4       4       4       - rw---    [ anon ]
FE940000       4       4       4       - rw---    [ anon ]
FEAD0000       4       4       4       - rwx--    [ anon ]
FEB70000    1260     692       -       - r-x--  libc.so.1
FECBB000      40      36      36       - rwx--  libc.so.1
FECC5000      16       8       8       - rwx--  libc.so.1
FEE80000     196     196       -       - r-x--  libumem.so.1
FEEC0000       8       8       8       - rwx--  libumem.so.1
FEED2000      76      72      72       - rw---  libumem.so.1
FEEE5000      24      24      24       - rw---  libumem.so.1
FEF20000       4       4       4       - rwx--    [ anon ]
FEF40000      24      12      12       - rwx--    [ anon ]
FEFB0000       4       4       -       -r--s-
FEFB4000     220     220       -       - r-x--  ld.so.1
FEFFB000       8       8       8       - rwx--  ld.so.1
FEFFD000       4       4       4       - rwx--  ld.so.1
-------- ------- ------- ------- -------
total Kb  104624    1636     516       -
#3

Updated by Electric Monk 2 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 80 to 100

git commit 6760681825872e5ab549ae9309431453fa97e428

commit  6760681825872e5ab549ae9309431453fa97e428
Author: Pavel Zakharov <pavel.zakharov@delphix.com>
Date:   2019-03-15T22:14:31.000Z

    10548 libumem doesn't free memory allocated by valloc or memalign
    Reviewed by: Andy Fiddaman <andy@omniosce.org>
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Marco van Wieringen <mvw@planets.elm.net>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF