Project

General

Profile

Actions

Bug #13989

closed

libproc can leak core CTF data

Added by Robert Mustacchi 3 months ago. Updated 2 months ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

While investigating issues with #13987 there was a second leak in addition to #13988 that was much more prevalent. If we fire it up in the same way as over there and look at its bufctls:

rm@beowulf:~/test$ mdb /usr/bin/amd64/pflags 
> _start_crt+0x87::bp
> ::run core.100560
...
> ::findleaks
CACHE             LEAKED           BUFCTL CALLER
0000000000b6d028       1 0000000000c48b60 libc.so.1`strdup+0x25
0000000000bc0028       1 0000000000e30540 libproc.so.1`core_load_shdrs+0x395
0000000000bc2028       1 0000000000e30460 libproc.so.1`core_load_shdrs+0x395
0000000000bc4028       1 0000000000e302a0 libproc.so.1`core_load_shdrs+0x395
0000000000bc9028       1 0000000000e30620 libproc.so.1`core_load_shdrs+0x395
0000000000bd7028       1 00000000012270e0 libproc.so.1`core_load_shdrs+0x395
0000000000bea028       1 0000000000e30380 libproc.so.1`core_load_shdrs+0x395
------------------------------------------------------------------------
           Total       7 buffers, 290832 bytes
> 0000000000e30540::bufctl -v
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
          e30540           c92000     13a545c342a3                1
                           bc0028                0                0
                 libumem.so.1`umem_cache_alloc_debug+0x104
                 libumem.so.1`umem_cache_alloc+0xd8
                 libumem.so.1`umem_alloc+0x9f
                 libumem.so.1`umem_malloc+0x36
                 libproc.so.1`core_load_shdrs+0x395
                 libproc.so.1`Pfgrab_core+0x891
                 libproc.so.1`proc_grab_common+0x1c3
                 libproc.so.1`proc_arg_xgrab+0x1d
                 look+0x52
                 main+0xcc
                 _start_crt+0x87
                 _start+0x18

> c92000,30::dump
         \/ 1 2 3  4 5 6 7  8 9 a b  c d e f  v123456789abcdef
c92000:  fecaddba fecaddba 0b270000 f548ba16  .........'...H..
c92010:  f1cf0201 00000000 00000000 00000000  ................
c92020:  08000000 28010000 b0060000 c4160000  ....(...........
> c92000,2::print malloc_data_t
{
    malloc_size = 0xbaddcafe
    malloc_stat = 0xbaddcafe
}
{
    malloc_size = 0x270b
    malloc_stat = 0x16ba48f5
}

If we look at the function in question, the only buffer that is nominally left behind would be a CTF related one as the others are freed. And if we look at the data here, at offstet +0x10 we have the CTF magic. In addition, since this was allocated by malloc we know the first 16 byes are in use with the two tags based on this size. The core file path is interesting here. In particular, we allocate the memory for the file pointer's ctf buffer independent from where we construct it. When we normally are building CTF we would immediately free the buffer on failure and not bother creating it unless we need it, but here we're allocating and reading this in eagerly from the core file, but in the case of say pfiles or pmap, they never request the CTF data themselves.

If we go to file_info_free, we only free the underlying data buffer if we actually have used this data with libctf. To lazily test this hypothesis, I just added an fprintf right above the free and verified that we had a data buffer, but not a ctf_file_t, confirming we hit this.

Thankfully the fix is simple, we just simply always free the data buffer regardless of whether or not we have a ctf_file_t.

Actions #1

Updated by Robert Mustacchi 2 months ago

To test this I reran various tests that caused this and found that findleaks was clean. I also used mdb and ptools in general with this change present.

Actions #2

Updated by Robert Mustacchi 2 months ago

  • Gerrit CR set to 1634
Actions #3

Updated by Electric Monk 2 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 9148a91fab5cca2666b01bab18514159f4480f9e

commit  9148a91fab5cca2666b01bab18514159f4480f9e
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-08-17T16:03:38.000Z

    13989 libproc can leak core CTF data
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF