From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by sourceware.org (Postfix) with ESMTPS id C0FAD383A379 for ; Thu, 21 Jul 2022 20:07:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C0FAD383A379 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f51.google.com with SMTP id bv24so3783742wrb.3 for ; Thu, 21 Jul 2022 13:07:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=1oySSr7B5usetvpzpNWnSXNNeIZK0jfVFcyIWJ1GJzQ=; b=GxLHqnttGzsdooMbJMeAu1PuQ6xCGzQVrEB261rT+DIiSoHWDOvomXTJfMThTcRDNb 9m3VI9v0SmVv1fcyawcB296kFF2E4YNyd3HR98KKIMRn+bfif+yt/2cyhaMdU2wWfa/x 8MJLW1HNZg4HBeXljKW90RDZ1YnNdgiiIImU3/W+i6sUTgZSlvGXO5bnP9NXByMqTKWv OCu3KaAWlOF6Z3gIQuyt/b3y7eybRr/gnBmTPMopElTg7e+IXHh/vUgoHbDexV/9pSaA WgAyBLSb1Oqa8UTLTVbweg5txj/GPSo+8Pxx1lvHvwSKr9hUV8KwX3paZux7UfQSfYyh cHjw== X-Gm-Message-State: AJIora+k/GwZ4L3UQRp/KN8p8bIh4op0jZM4z8zNRHfu4XKCHxmV+YRt +3zzEqxSHWd62Y2+1bFJMgtia9UjzqI= X-Google-Smtp-Source: AGRyM1u1/CEcpqwpQdNdSNiD3L+cvXWwGgCjBE9d7fwAFbgmOAgV3BaeW2hJ33xYCkNoun+ruwgGlg== X-Received: by 2002:adf:f412:0:b0:21d:8aa4:e796 with SMTP id g18-20020adff412000000b0021d8aa4e796mr67212wro.79.1658434067814; Thu, 21 Jul 2022 13:07:47 -0700 (PDT) Received: from ?IPv6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id j13-20020a5d448d000000b002167efdd549sm2729593wrq.38.2022.07.21.13.07.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 21 Jul 2022 13:07:47 -0700 (PDT) Subject: Re: [PATCH] linux_nat_target::xfer_partial: Fallback to ptrace To: Keith Seitz , gdb-patches@sourceware.org References: <20220603151839.2632317-1-keiths@redhat.com> From: Pedro Alves Message-ID: <97900ea5-350c-c30c-9a55-98355e8f9abc@palves.net> Date: Thu, 21 Jul 2022 21:07:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.7 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Jul 2022 20:07:53 -0000 Hi Keith, On 2022-07-21 4:03 p.m., Keith Seitz wrote: > Ping Appologies for the delay, there was something here that I wanted to think about, and then it just fell through the cracks. See below. > On 6/3/22 08:18, Keith Seitz via Gdb-patches wrote: >> Pedro Alves wrote: >> >>> I guess the write to /proc/pid/mem fails with EIO for you, and there's nothing >>> else we can use to detect the scenario.  So we probably want to check TARGET_XFER_E_IO >>> instead.  And, maybe only do the fallback if writing. >> >> I've updated the patch to include these changes and retested on all the "usual" >> target boards on Fedora 36 x86_64, s390x, ppc64le, and aarch64 and on RHEL6 >> x86_64. Thanks, that's a lot of testing. >> Commit 05c06f318fd9a112529dfc313e6512b399a645e4 enabled GDB >> to access memory while threads are running. It did this by accessing >> /proc/PID/task/LWP/mem. >> >> Unfortunately, this interface is not implemented for writing in older kernels >> (such as RHEL6). This means that GDB is unable to insert breakpoints on >> these hosts: >> >> $ ./gdb -q gdb -ex start >> Reading symbols from gdb... >> Temporary breakpoint 1 at 0x40fdd5: file ../../src/gdb/gdb.c, line 28. >> Starting program: /home/rhel6/fsf/linux/gdb/gdb >> Warning: >> Cannot insert breakpoint 1. >> Cannot access memory at address 0x40fdd5 >> >> (gdb) >> >> Before this patch, linux_proc_xfer_memory_partial (previously called >> linux_proc_xfer_partial) would return TARGET_XFER_EOF if the write >> to /proc/PID/mem failed. [More specifically, linux_proc_xfer_partial would >> not "bother for one word," but the effect is the essentially same.] >> >> This status was checked by linux_nat_target::xfer_partial, which would then >> fallback to using ptrace to perform the operation. >> >> This is the specific hunk that removed the fallback: >> >> -  xfer = linux_proc_xfer_partial (object, annex, readbuf, writebuf, >> -                                 offset, len, xfered_len); >> -  if (xfer != TARGET_XFER_EOF) >> -    return xfer; >> +      return linux_proc_xfer_memory_partial (readbuf, writebuf, >> +                                            offset, len, xfered_len); >> +    } >> >>     return inf_ptrace_target::xfer_partial (object, annex, readbuf, writebuf, >>                                            offset, len, xfered_len); >> >> This patch restores this fallback mechanism, enabling GDB to insert >> breakpoints on these older kernels. Note that a recent patch changed >> the return status from TARGET_XFER_EOF to TARGET_XFER_E_IO. >> >> Tested on {unix,native-gdbserver,native-extended-gdbserver}/-m{32,64} >> on x86_64, s390x, aarch64, and ppc64le. >> --- >>   gdb/linux-nat.c | 8 ++++++-- >>   1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c >> index 3b5400896bc..571c97137c2 100644 >> --- a/gdb/linux-nat.c >> +++ b/gdb/linux-nat.c >> @@ -3706,8 +3706,12 @@ linux_nat_target::xfer_partial (enum target_object object, >>         if (addr_bit < (sizeof (ULONGEST) * HOST_CHAR_BIT)) >>       offset &= ((ULONGEST) 1 << addr_bit) - 1; >>   -      return linux_proc_xfer_memory_partial (readbuf, writebuf, >> -                         offset, len, xfered_len); >> +      enum target_xfer_status xfer >> +    = linux_proc_xfer_memory_partial (readbuf, writebuf, >> +                      offset, len, xfered_len); >> +      if (xfer != TARGET_XFER_E_IO || readbuf != nullptr) >> +    return xfer; >> +      /* Fallthrough to ptrace.  /proc/pid/mem wasn't writable before Linux 2.6.39.  */ >>       } >>       return inf_ptrace_target::xfer_partial (object, annex, readbuf, writebuf, > So my worry here is this bringing back a race with more modern kernels, one which the always writing via /proc/pid/mem plugged -- when the inferior execs, writes through /proc/pid/mem fail, as the old pre-exec address space is gone. Only when we see the exec even out of ptrace, will we close the file and reopen a new one to access the post-exec address space. Up until recent kernels, ptrace calls didn't have a similar protection. I.e., if the inferior execs, and the write through /proc/pid/mem fails because of an exec, and then we fallback to ptrace, that write will succeed, but it will be writing bytes in the post-exec address space that were meant for the pre-exec address space. So I was wondering about mitigating this by only falling back to ptrace if writing to /proc/pid/mem doesn't really work. Checking the kernel version itself seems a bit fragile, so I thought we could make gdb probe once at started up whether writing to itself via /proc/self/mem works. It turns out that actually works. With this, you'd just add an extra proc_mem_file_is_writable() check in your patch before falling back, or even, skip straight to ptrace if !proc_mem_file_is_writable(). WDYT? >From 56622b9cadff4b62a0b05861015ce06cf9d6e8f2 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 21 Jul 2022 19:11:16 +0100 Subject: [PATCH] gdb/linux-nat: Check whether /proc/pid/mem is writable Probe whether /proc/pid/mem is writable, by using it to write to a GDB variable. Change-Id: If87eff0b46cbe5e32a583e2977a9e17d29d0ed3e --- gdb/linux-nat.c | 106 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 89 insertions(+), 17 deletions(-) diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 0a93ab5c6ae..0916b841ec7 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -3674,6 +3674,8 @@ static enum target_xfer_status linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, ULONGEST offset, LONGEST len, ULONGEST *xfered_len); +static bool proc_mem_file_is_writable (); + enum target_xfer_status linux_nat_target::xfer_partial (enum target_object object, const char *annex, gdb_byte *readbuf, @@ -3882,25 +3884,19 @@ open_proc_mem_file (ptid_t ptid) fd, ptid.pid (), ptid.lwp ()); } -/* Implement the to_xfer_partial target method using /proc/PID/mem. - Because we can use a single read/write call, this can be much more - efficient than banging away at PTRACE_PEEKTEXT. Also, unlike - PTRACE_PEEKTEXT/PTRACE_POKETEXT, this works with running - threads. */ +/* Helper for linux_proc_xfer_memory_partial and + proc_mem_file_is_writable. FD is the already opened /proc/pid/mem + file, and PID is the pid of the corresponding process. The rest of + the arguments are like linux_proc_xfer_memory_partial's. */ static enum target_xfer_status -linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, - ULONGEST offset, LONGEST len, - ULONGEST *xfered_len) +linux_proc_xfer_memory_partial_fd (int fd, int pid, + gdb_byte *readbuf, const gdb_byte *writebuf, + ULONGEST offset, LONGEST len, + ULONGEST *xfered_len) { ssize_t ret; - auto iter = proc_mem_file_map.find (inferior_ptid.pid ()); - if (iter == proc_mem_file_map.end ()) - return TARGET_XFER_EOF; - - int fd = iter->second.fd (); - gdb_assert (fd != -1); /* Use pread64/pwrite64 if available, since they save a syscall and can @@ -3919,8 +3915,7 @@ linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, if (ret == -1) { linux_nat_debug_printf ("accessing fd %d for pid %d failed: %s (%d)", - fd, inferior_ptid.pid (), - safe_strerror (errno), errno); + fd, pid, safe_strerror (errno), errno); return TARGET_XFER_E_IO; } else if (ret == 0) @@ -3928,7 +3923,7 @@ linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, /* EOF means the address space is gone, the whole process exited or execed. */ linux_nat_debug_printf ("accessing fd %d for pid %d got EOF", - fd, inferior_ptid.pid ()); + fd, pid); return TARGET_XFER_EOF; } else @@ -3938,6 +3933,81 @@ linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, } } +/* Implement the to_xfer_partial target method using /proc/PID/mem. + Because we can use a single read/write call, this can be much more + efficient than banging away at PTRACE_PEEKTEXT. Also, unlike + PTRACE_PEEKTEXT/PTRACE_POKETEXT, this works with running + threads. */ + +static enum target_xfer_status +linux_proc_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, + ULONGEST offset, LONGEST len, + ULONGEST *xfered_len) +{ + int pid = inferior_ptid.pid (); + + auto iter = proc_mem_file_map.find (pid); + if (iter == proc_mem_file_map.end ()) + return TARGET_XFER_EOF; + + int fd = iter->second.fd (); + + return linux_proc_xfer_memory_partial_fd (fd, pid, readbuf, writebuf, offset, + len, xfered_len); +} + +/* Check whether /proc/pid/mem is writable in the current kernel, and + return true if so. It wasn't writable before Linux 2.6.39, but + there's no way to know whether the feature was backported to older + kernels. So we check to see if it works. The result is cached, + and this is garanteed to be called once early at startup. */ + +static bool +proc_mem_file_is_writable () +{ + static gdb::optional writable; + + if (writable.has_value ()) + return *writable; + + *writable = false; + + /* We check whether /proc/pid/mem is writable by trying to write to + one of our variables via /proc/self/mem. */ + + int fd = gdb_open_cloexec ("/proc/self/mem", O_RDWR | O_LARGEFILE, 0).release (); + + if (fd == -1) + { + warning (_("opening /proc/self/mem file failed: %s (%d)"), + safe_strerror (errno), errno); + return *writable; + } + + SCOPE_EXIT { close (fd); }; + + /* This is the variable we try to write to. Note OFFSET below. */ + volatile static gdb_byte test_var = 0; + + gdb_byte writebuf[] = {0x55}; + ULONGEST offset = (uintptr_t) &test_var; + ULONGEST xfered_len; + + enum target_xfer_status res + = linux_proc_xfer_memory_partial_fd (fd, getpid (), nullptr, writebuf, + offset, 1, &xfered_len); + + if (res == TARGET_XFER_OK) + { + gdb_assert (xfered_len == 1); + gdb_assert (test_var == 0x55); + /* Success. */ + *writable = true; + } + + return *writable; +} + /* Parse LINE as a signal set and add its set bits to SIGS. */ static void @@ -4437,6 +4507,8 @@ Enables printf debugging output."), sigemptyset (&blocked_mask); lwp_lwpid_htab_create (); + + proc_mem_file_is_writable (); } base-commit: c07ec968f7342a2386969bc192ff0b42e33991ef -- 2.36.0