Skip to content

Commit

Permalink
gdbserver/linux: free process_info_private and arch_process_info when…
Browse files Browse the repository at this point in the history
… failing to attach

Running

  $ ../gdbserver/gdbserver --once --attach :1234 539436

with ASan while /proc/sys/kernel/yama/ptrace_scope is set to 1 (prevents
attaching) shows that we fail to free some platform-specific objects
tied to the process_info (process_info_private and arch_process_info):

    Direct leak of 32 byte(s) in 1 object(s) allocated from:
        #0 0x7f6b558b3fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x562eaf15d04a in xcalloc /home/simark/src/binutils-gdb/gdbserver/../gdb/alloc.c:100
        #2 0x562eaf251548 in xcnew<process_info_private> /home/simark/src/binutils-gdb/gdbserver/../gdbsupport/poison.h:122
        #3 0x562eaf22810c in linux_process_target::add_linux_process_no_mem_file(int, int) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:426
        #4 0x562eaf22d33f in linux_process_target::attach(unsigned long) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:1132
        #5 0x562eaf1a7222 in attach_inferior /home/simark/src/binutils-gdb/gdbserver/server.cc:308
        #6 0x562eaf1c1016 in captured_main /home/simark/src/binutils-gdb/gdbserver/server.cc:3949
        #7 0x562eaf1c1d60 in main /home/simark/src/binutils-gdb/gdbserver/server.cc:4084
        #8 0x7f6b552f630f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)

    Indirect leak of 56 byte(s) in 1 object(s) allocated from:
        #0 0x7f6b558b3fb9 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x562eaf15d04a in xcalloc /home/simark/src/binutils-gdb/gdbserver/../gdb/alloc.c:100
        #2 0x562eaf2a0d79 in xcnew<arch_process_info> /home/simark/src/binutils-gdb/gdbserver/../gdbsupport/poison.h:122
        #3 0x562eaf295e2c in x86_target::low_new_process() /home/simark/src/binutils-gdb/gdbserver/linux-x86-low.cc:723
        #4 0x562eaf22819b in linux_process_target::add_linux_process_no_mem_file(int, int) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:428
        #5 0x562eaf22d33f in linux_process_target::attach(unsigned long) /home/simark/src/binutils-gdb/gdbserver/linux-low.cc:1132
        #6 0x562eaf1a7222 in attach_inferior /home/simark/src/binutils-gdb/gdbserver/server.cc:308
        #7 0x562eaf1c1016 in captured_main /home/simark/src/binutils-gdb/gdbserver/server.cc:3949
        #8 0x562eaf1c1d60 in main /home/simark/src/binutils-gdb/gdbserver/server.cc:4084
        #9 0x7f6b552f630f in __libc_start_call_main (/usr/lib/libc.so.6+0x2d30f)

Those objects are deleted by linux_process_target::mourn, but that is
not called if we fail to attach, we only call remove_process.  I
initially fixed this by making linux_process_target::attach call
linux_process_target::mourn on failure (before calling error).  But this
isn't done anywhere else (including in GDB) so it would just be
confusing to do things differently here.

Instead, add a linux_process_target::remove_linux_process helper method
(which calls remove_process), and call that instead of remove_process in
the Linux target.  Move the free-ing of the extra data from the mourn
method to that new method.

Change-Id: I277059a69d5f08087a7f3ef0b8f1792a1fcf7a85
  • Loading branch information
simark committed Apr 22, 2022
1 parent 91395d9 commit f551c8e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
27 changes: 16 additions & 11 deletions gdbserver/linux-low.cc
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,20 @@ linux_process_target::add_linux_process (int pid, int attached)
return proc;
}

void
linux_process_target::remove_linux_process (process_info *proc)
{
if (proc->priv->mem_fd >= 0)
close (proc->priv->mem_fd);

this->low_delete_process (proc->priv->arch_private);

xfree (proc->priv);
proc->priv = nullptr;

remove_process (proc);
}

arch_process_info *
linux_process_target::low_new_process ()
{
Expand Down Expand Up @@ -1136,7 +1150,7 @@ linux_process_target::attach (unsigned long pid)
err = attach_lwp (ptid);
if (err != 0)
{
remove_process (proc);
this->remove_linux_process (proc);

std::string reason = linux_ptrace_attach_fail_reason_string (ptid, err);
error ("Cannot attach to process %ld: %s", pid, reason.c_str ());
Expand Down Expand Up @@ -1565,8 +1579,6 @@ linux_process_target::detach (process_info *process)
void
linux_process_target::mourn (process_info *process)
{
struct process_info_private *priv;

#ifdef USE_THREAD_DB
thread_db_mourn (process);
#endif
Expand All @@ -1576,14 +1588,7 @@ linux_process_target::mourn (process_info *process)
delete_lwp (get_thread_lwp (thread));
});

/* Freeing all private data. */
priv = process->priv;
close (priv->mem_fd);
low_delete_process (priv->arch_private);
free (priv);
process->priv = NULL;

remove_process (process);
this->remove_linux_process (process);
}

void
Expand Down
3 changes: 3 additions & 0 deletions gdbserver/linux-low.h
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,9 @@ class linux_process_target : public process_stratum_target
yet. */
process_info *add_linux_process_no_mem_file (int pid, int attached);

/* Free resources associated to PROC and remove it. */
void remove_linux_process (process_info *proc);

/* Add a new thread. */
lwp_info *add_lwp (ptid_t ptid);

Expand Down

0 comments on commit f551c8e

Please sign in to comment.