From: Daniel Axtens <dja@axtens•net>
To: Radu Rendec <radu.rendec@gmail•com>, linuxppc-dev@lists•ozlabs.org
Cc: Radu Rendec <radu.rendec@gmail•com>,
Paul Mackerras <paulus@samba•org>,
Oleg Nesterov <oleg@redhat•com>
Subject: Re: [PATCH 0/1] PPC32: fix ptrace() access to FPU registers
Date: Mon, 17 Jun 2019 11:19:39 +1000 [thread overview]
Message-ID: <87r27t2el0.fsf@dja-thinkpad.axtens.net> (raw)
In-Reply-To: <20190610232758.19010-1-radu.rendec@gmail.com>
Radu Rendec <radu.rendec@gmail•com> writes:
> Hi Everyone,
>
> I'm following up on the ptrace() problem that I reported a few days ago.
> I believe my version of the code handles all cases correctly. While the
> problem essentially boils down to dividing the fpidx by 2 on PPC32, it
> becomes tricky when the same code must work correctly on both PPC32 and
> PPC64.
>
> One other thing that I believe was handled incorrectly in the previous
> version is the unused half of fpscr on PPC32. Note that while PT_FPSCR
> is defined as (PT_FPR0 + 2*32 + 1), making only the upper half visible,
> PT_FPR0 + 2*32 still corresponds to a possible address that userspace
> can pass. In that case, comparing fpidx to (PT_FPSCR - PT_FPR0) would
> cause an invalid access to the FPU registers array.
>
> I tested the patch on 4.9.179, but that part of the code is identical in
> recent kernels so it should work just the same.
>
> I wrote a simple test program than can be used to quickly test (on an
> x86_64 host) that all cases are handled correctly for both PPC32/PPC64.
> The code is included below.
>
> I also tested with gdbserver (test patch included below) and verified
> that it generates two ptrace() calls for each FPU register, with
> addresses between 0xc0 and 0x1bc.
Thanks for looking in to this. I can confirm your issue. What I'm
currently wondering is: what is the behaviour with a 32-bit userspace on
a 64-bit kernel? Should they also be going down the 32-bit path as far
as calculating offsets goes?
Regards,
Daniel
>
> 8<--------------- Makefile ---------------------------------------------
> .PHONY: all clean
>
> all: ptrace-fpregs-32 ptrace-fpregs-64
>
> ptrace-fpregs-32: ptrace-fpregs.c
> $(CC) -o ptrace-fpregs-32 -Wall -O2 -m32 ptrace-fpregs.c
>
> ptrace-fpregs-64: ptrace-fpregs.c
> $(CC) -o ptrace-fpregs-64 -Wall -O2 ptrace-fpregs.c
>
> clean:
> rm -f ptrace-fpregs-32 ptrace-fpregs-64
> 8<--------------- ptrace-fpregs.c --------------------------------------
> #include <stdio.h>
> #include <errno.h>
>
> #define PT_FPR0 48
>
> #ifndef __x86_64
>
> #define PT_FPR31 (PT_FPR0 + 2*31)
> #define PT_FPSCR (PT_FPR0 + 2*32 + 1)
>
> #else
>
> #define PT_FPSCR (PT_FPR0 + 32)
>
> #endif
>
> int test_access(unsigned long addr)
> {
> int ret;
>
> do {
> unsigned long index, fpidx;
>
> ret = -EIO;
>
> /* convert to index and check */
> index = addr / sizeof(long);
> if ((addr & (sizeof(long) - 1)) || (index > PT_FPSCR))
> break;
>
> if (index < PT_FPR0) {
> ret = printf("ptrace_put_reg(%lu)", index);
> break;
> }
>
> ret = 0;
> #ifndef __x86_64
> if (index == PT_FPSCR - 1) {
> /* corner case for PPC32; do nothing */
> printf("corner_case");
> break;
> }
> #endif
> if (index == PT_FPSCR) {
> printf("fpscr");
> break;
> }
>
> /*
> * FPR is always 64-bit; on PPC32, userspace does two 32-bit
> * accesses. Add bit2 to allow accessing the upper half on
> * 32-bit; on 64-bit, bit2 is always 0 (we validate it above).
> */
> fpidx = (addr - PT_FPR0 * sizeof(long)) / 8;
> printf("TS_FPR[%lu] + %lu", fpidx, addr & 4);
> break;
> } while (0);
>
> return ret;
> }
>
> int main(void)
> {
> unsigned long addr;
> int rc;
>
> for (addr = 0; addr < PT_FPSCR * sizeof(long) + 16; addr++) {
> printf("0x%04lx: ", addr);
> rc = test_access(addr);
> if (rc < 0)
> printf("!err!");
> printf("\t<%d>\n", rc);
> }
>
> return 0;
> }
> 8<--------------- gdb.patch --------------------------------------------
> --- gdb/gdbserver/linux-low.c.orig 2019-06-10 11:45:53.810882669 -0400
> +++ gdb/gdbserver/linux-low.c 2019-06-10 11:49:32.272929766 -0400
> @@ -4262,6 +4262,8 @@ store_register (struct regcache *regcach
> pid = lwpid_of (get_thread_lwp (current_inferior));
> for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
> {
> + printf("writing register #%d offset %d at address %#x\n",
> + regno, i, (unsigned int)regaddr);
> errno = 0;
> ptrace (PTRACE_POKEUSER, pid,
> /* Coerce to a uintptr_t first to avoid potential gcc warning
> 8<----------------------------------------------------------------------
>
> Radu Rendec (1):
> PPC32: fix ptrace() access to FPU registers
>
> arch/powerpc/kernel/ptrace.c | 85 ++++++++++++++++++++++--------------
> 1 file changed, 52 insertions(+), 33 deletions(-)
>
> --
> 2.20.1
next prev parent reply other threads:[~2019-06-17 1:21 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-10 23:27 [PATCH 0/1] PPC32: fix ptrace() access to FPU registers Radu Rendec
2019-06-10 23:27 ` [PATCH 1/1] " Radu Rendec
2019-06-13 7:59 ` [PATCH 0/1] " Daniel Axtens
2019-06-17 1:19 ` Daniel Axtens [this message]
2019-06-17 2:27 ` Radu Rendec
2019-06-18 6:42 ` Daniel Axtens
2019-06-18 12:16 ` Radu Rendec
[not found] ` <fbf9f9cbb99fc40c7d7af86fee3984427c61b799.camel__46559.9162316479$1560860409$gmane$org@gmail.com>
2019-06-18 18:09 ` Andreas Schwab
2019-06-19 0:36 ` Daniel Axtens
2019-06-19 12:57 ` Radu Rendec
2021-06-11 6:02 ` Christophe Leroy
2021-06-11 14:37 ` Radu Rendec
2021-07-18 18:07 ` Radu Rendec
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r27t2el0.fsf@dja-thinkpad.axtens.net \
--to=dja@axtens$(echo .)net \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=oleg@redhat$(echo .)com \
--cc=paulus@samba$(echo .)org \
--cc=radu.rendec@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox