From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1879) id 5B5DF3858D37; Fri, 22 Apr 2022 18:04:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5B5DF3858D37 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Simon Marchi To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdbserver/linux: free process_info_private and arch_process_info when failing to attach X-Act-Checkin: binutils-gdb X-Git-Author: Simon Marchi X-Git-Refname: refs/heads/master X-Git-Oldrev: 91395d97d905c31ac38513e4aaedecb3b25e818f X-Git-Newrev: f551c8ef326a022ec44279e66ed9734692225067 Message-Id: <20220422180445.5B5DF3858D37@sourceware.org> Date: Fri, 22 Apr 2022 18:04:45 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Apr 2022 18:04:45 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Df551c8ef326a= 022ec44279e66ed9734692225067 commit f551c8ef326a022ec44279e66ed9734692225067 Author: Simon Marchi Date: Fri Apr 22 11:34:54 2022 -0400 gdbserver/linux: free process_info_private and arch_process_info when f= ailing to attach =20 Running =20 $ ../gdbserver/gdbserver --once --attach :1234 539436 =20 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): =20 Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7f6b558b3fb9 in __interceptor_calloc /usr/src/debug/gcc/li= bsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x562eaf15d04a in xcalloc /home/simark/src/binutils-gdb/gdbs= erver/../gdb/alloc.c:100 #2 0x562eaf251548 in xcnew /home/simark/s= rc/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-gd= b/gdbserver/server.cc:3949 #7 0x562eaf1c1d60 in main /home/simark/src/binutils-gdb/gdbserv= er/server.cc:4084 #8 0x7f6b552f630f in __libc_start_call_main (/usr/lib/libc.so.6= +0x2d30f) =20 Indirect leak of 56 byte(s) in 1 object(s) allocated from: #0 0x7f6b558b3fb9 in __interceptor_calloc /usr/src/debug/gcc/li= bsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x562eaf15d04a in xcalloc /home/simark/src/binutils-gdb/gdbs= erver/../gdb/alloc.c:100 #2 0x562eaf2a0d79 in xcnew /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-gd= b/gdbserver/server.cc:3949 #8 0x562eaf1c1d60 in main /home/simark/src/binutils-gdb/gdbserv= er/server.cc:4084 #9 0x7f6b552f630f in __libc_start_call_main (/usr/lib/libc.so.6= +0x2d30f) =20 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. =20 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. =20 Change-Id: I277059a69d5f08087a7f3ef0b8f1792a1fcf7a85 Diff: --- gdbserver/linux-low.cc | 27 ++++++++++++++++----------- gdbserver/linux-low.h | 3 +++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index 449c6641f58..8b8614f6ed4 100644 --- a/gdbserver/linux-low.cc +++ b/gdbserver/linux-low.cc @@ -440,6 +440,20 @@ linux_process_target::add_linux_process (int pid, int = attached) return proc; } =20 +void +linux_process_target::remove_linux_process (process_info *proc) +{ + if (proc->priv->mem_fd >=3D 0) + close (proc->priv->mem_fd); + + this->low_delete_process (proc->priv->arch_private); + + xfree (proc->priv); + proc->priv =3D nullptr; + + remove_process (proc); +} + arch_process_info * linux_process_target::low_new_process () { @@ -1136,7 +1150,7 @@ linux_process_target::attach (unsigned long pid) err =3D attach_lwp (ptid); if (err !=3D 0) { - remove_process (proc); + this->remove_linux_process (proc); =20 std::string reason =3D linux_ptrace_attach_fail_reason_string (ptid,= err); error ("Cannot attach to process %ld: %s", pid, reason.c_str ()); @@ -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 @@ -1576,14 +1588,7 @@ linux_process_target::mourn (process_info *process) delete_lwp (get_thread_lwp (thread)); }); =20 - /* Freeing all private data. */ - priv =3D process->priv; - close (priv->mem_fd); - low_delete_process (priv->arch_private); - free (priv); - process->priv =3D NULL; - - remove_process (process); + this->remove_linux_process (process); } =20 void diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h index 4284fb3348a..79be31b8f72 100644 --- a/gdbserver/linux-low.h +++ b/gdbserver/linux-low.h @@ -547,6 +547,9 @@ private: yet. */ process_info *add_linux_process_no_mem_file (int pid, int attached); =20 + /* Free resources associated to PROC and remove it. */ + void remove_linux_process (process_info *proc);=20 + /* Add a new thread. */ lwp_info *add_lwp (ptid_t ptid);