From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 0115D3857031 for ; Wed, 5 Jul 2023 17:59:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0115D3857031 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1688579951; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=6EK2DyM4Aj1Gi+9OnGwmi89w8d8k4DYXDTb059IOe1I=; b=IpysI2cFyxIuSWk+j1oP0i1s3LDiEZeLpGEoB6e38iXWVp91yvkma5kDc1aH5pKBEIuNC+ wKvpGuVQaASwqVXKnRf48pHwyXAUAHHKFKt7Xa20+Sk/hQr7wpdoaeD4OPZgdG6jjb5okJ wfBtEhCxNL9XpY41BsZrsiS0DnKLwyw= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-638-XBGgTmaTN8aXJ-K9JcXjgQ-1; Wed, 05 Jul 2023 13:59:10 -0400 X-MC-Unique: XBGgTmaTN8aXJ-K9JcXjgQ-1 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-3fa9a282fffso40868905e9.1 for ; Wed, 05 Jul 2023 10:59:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688579949; x=1691171949; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6EK2DyM4Aj1Gi+9OnGwmi89w8d8k4DYXDTb059IOe1I=; b=U+F2sDz0Vy/Bh2eabl35Q6kPLRQvoP93xZjWqzIzd9sNQ9fdzsXSCkpGWATbBBwdcR fnVbz4223O+tzNTHAmKm9BWLUY1NskjcnCV+gDSOA/0YyY8MpdaV7NFvbhQNOQaDS+5V kc8OPKTJCOWIPIMvIPSfeLYjNGaIxxivDEVUZK4fgxxK1OGx7PrUSYuJC8huvE5HuLkC zrKn18/4h226/Dsp6HwjJXyhkELWGD7oyPxeTyT57C7XqA4vaiJdHBCMQkKSbL3jy8R7 d9yTY20JrnbwNCM/lbCyj5CWz70bPmY/dcnt8E6G3qN83Wl1acpLPw10AZwV1L2s+4Q5 Cnew== X-Gm-Message-State: AC+VfDzg0s8G/tib7GHAIp1oaZPLKyocZGcHH2QQMKeD2CFtrLfvj0zq tE/yjp2zBUFclE/28BwYO9gyd+kmrX5tvibexs3ZN60QmHB+8WbX4rFHPwSvEFzKdZ7/1kWNfuT VkLD5c/7xbdrCjoUOa/zeDg== X-Received: by 2002:a05:600c:b54:b0:3fb:40ce:503e with SMTP id k20-20020a05600c0b5400b003fb40ce503emr13667545wmr.25.1688579949379; Wed, 05 Jul 2023 10:59:09 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7xGnz7y3xIDyoOSBN2L8JP7bhCvAiXXPh6Eoc6RTeSjh66NUB1XFVXX5iRPtKviNSZkP6Q5g== X-Received: by 2002:a05:600c:b54:b0:3fb:40ce:503e with SMTP id k20-20020a05600c0b5400b003fb40ce503emr13667532wmr.25.1688579949015; Wed, 05 Jul 2023 10:59:09 -0700 (PDT) Received: from localhost (2.72.115.87.dyn.plus.net. [87.115.72.2]) by smtp.gmail.com with ESMTPSA id n5-20020a05600c294500b003fbc0a49b57sm2822962wmd.6.2023.07.05.10.59.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Jul 2023 10:59:08 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , gdb-patches@sourceware.org Cc: Matt Turner Subject: Re: [PATCH] Linux: Avoid pread64/pwrite64 for high memory addresses (PR gdb/30525) In-Reply-To: <20230705134141.1441753-1-pedro@palves.net> References: <20230705134141.1441753-1-pedro@palves.net> Date: Wed, 05 Jul 2023 18:59:07 +0100 Message-ID: <87a5wakzo4.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Pedro Alves writes: > Since commit 05c06f318fd9 ("Linux: Access memory even if threads are > running"), GDB prefers pread64/pwrite64 to access inferior memory > instead of ptrace. That change broke reading shared libraries on > SPARC Linux, as reported by PR gdb/30525 ("gdb cannot read shared > libraries on SPARC64"). > > On SPARC64 Linux, surprisingly (to me), userspace shared libraries are > mapped at high 64-bit addresses: > > (gdb) info sharedlibrary > Cannot access memory at address 0xfff80001002011e0 > Cannot access memory at address 0xfff80001002011d8 > Cannot access memory at address 0xfff80001002011d8 > From To Syms Read Shared Object Library > 0xfff80001000010a0 0xfff8000100021f80 Yes (*) /lib64/ld-linux.so.2 > (*): Shared library is missing debugging information. > > Those addresses are 64-bit addresses with the high bits set. When > interpreted as signed, they're negative. > > The Linux kernel rejects pread64/pwrite64 if the offset argument of > type off_t (a signed type) is negative, which happens if the memory > address we're accessing has its high bit set. See > linux/fs/read_write.c sys_pread64 and sys_pwrite64 in Linux. > > Thanksfully, lseek does not fail in that situation. So the fix is to > use the 'lseek + read|write' path if the offset would be negative. > > Fix this in both native GDB and GDBserver. > > Tested on a SPARC64 GNU/Linux and x86_64 GNU/Linux. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30525 > Change-Id: I79c724f918037ea67b7396fadb521bc9d1b10dc5 > --- > gdb/linux-nat.c | 30 ++++++++++++++++++++---------- > gdbserver/linux-low.cc | 33 ++++++++++++++++++++------------- > 2 files changed, 40 insertions(+), 23 deletions(-) > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index 383ef58fa23..c5efacbb3fa 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -3909,18 +3909,28 @@ linux_proc_xfer_memory_partial_fd (int fd, int pid, > > gdb_assert (fd != -1); > > - /* Use pread64/pwrite64 if available, since they save a syscall and can > - handle 64-bit offsets even on 32-bit platforms (for instance, SPARC > - debugging a SPARC64 application). */ > + /* Use pread64/pwrite64 if available, since they save a syscall and > + can handle 64-bit offsets even on 32-bit platforms (for instance, > + SPARC debugging a SPARC64 application). But only use them if the > + offset isn't so high that when cast to off_t it'd be negative, as > + seen on SPARC64. pread64/pwrite64 outright reject such offsets. > + lseek does not. */ > #ifdef HAVE_PREAD64 > - ret = (readbuf ? pread64 (fd, readbuf, len, offset) > - : pwrite64 (fd, writebuf, len, offset)); > -#else > - ret = lseek (fd, offset, SEEK_SET); > - if (ret != -1) > - ret = (readbuf ? read (fd, readbuf, len) > - : write (fd, writebuf, len)); > + if ((off_t) offset >= 0) > + { > + ret = (readbuf != nullptr > + ? pread64 (fd, readbuf, len, offset) > + : pwrite64 (fd, writebuf, len, offset)); > + } I haven't tested this, but the change looks good to me with one nit... I think the '{' and '}' here (and in gdbserver below) aren't GDB style, as there's only a single statement? Otherwise, Approved-By: Andrew Burgess thanks, Andrew > + else > #endif > + { > + ret = lseek (fd, offset, SEEK_SET); > + if (ret != -1) > + ret = (readbuf > + ? read (fd, readbuf, len) > + : write (fd, writebuf, len)); > + } > > if (ret == -1) > { > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index 8ab16698632..63668d81b45 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -5377,21 +5377,28 @@ proc_xfer_memory (CORE_ADDR memaddr, unsigned char *readbuf, > { > int bytes; > > - /* If pread64 is available, use it. It's faster if the kernel > - supports it (only one syscall), and it's 64-bit safe even on > - 32-bit platforms (for instance, SPARC debugging a SPARC64 > - application). */ > + /* Use pread64/pwrite64 if available, since they save a syscall > + and can handle 64-bit offsets even on 32-bit platforms (for > + instance, SPARC debugging a SPARC64 application). But only > + use them if the offset isn't so high that when cast to off_t > + it'd be negative, as seen on SPARC64. pread64/pwrite64 > + outright reject such offsets. lseek does not. */ > #ifdef HAVE_PREAD64 > - bytes = (readbuf != nullptr > - ? pread64 (fd, readbuf, len, memaddr) > - : pwrite64 (fd, writebuf, len, memaddr)); > -#else > - bytes = -1; > - if (lseek (fd, memaddr, SEEK_SET) != -1) > - bytes = (readbuf != nullptr > - ? read (fd, readbuf, len) > - : write (fd, writebuf, len)); > + if ((off_t) memaddr >= 0) > + { > + bytes = (readbuf != nullptr > + ? pread64 (fd, readbuf, len, memaddr) > + : pwrite64 (fd, writebuf, len, memaddr)); > + } > + else > #endif > + { > + bytes = -1; > + if (lseek (fd, memaddr, SEEK_SET) != -1) > + bytes = (readbuf != nullptr > + ? read (fd, readbuf, len) > + : write (fd, writebuf, len)); > + } > > if (bytes < 0) > return errno; > > base-commit: a5042543d819df8a76ebec8c4d715f244efbab0a > -- > 2.34.1