[Bug gdb/28947] New: GDB does not remove AArch64 pointer signatures before doing memory accesses

[Bug gdb/28947] New: GDB does not remove AArch64 pointer signatures before doing memory accesses

[david.spickett at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

            Bug ID: 28947
           Summary: GDB does not remove AArch64 pointer signatures before
                    doing memory accesses
           Product: gdb
           Version: HEAD
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: david.spickett at linaro dot org
  Target Milestone: ---

I'm using a somewhat recent build from source:
```
$ ./gdb --version
GNU gdb (GDB) 12.0.50.20220208-git
```

binutils @ 481153777e278b71e694fd2db6b897f7a9e3dcb8

Compile the following:
```
#include <stddef.h>

int main(int argc, char const *argv[]) {
  char* buf[16];

#define sign_ptr(ptr) \
  __asm__ __volatile__("pacdza %0" \
                       : "=r"(ptr) \
                       : "r"(ptr))

  // Set top byte including where memory tag bits would go.
  char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
  sign_ptr(buf_with_non_address);
  // Address is now:
  // <4 bit user tag><4 bit memory tag><pointer signature><virtual address>

  return 0; // Set break point at this line.
}
```

With:
```
aarch64-unknown-linux-gnu-gcc -march=armv8.3-a main.c -o prog -g
```

Then run it on an AArch64 machine that has pointer authentication enabled.
Break at the indicated line where main returns.

Expected behaviour: GDB would know that pointer signatures are "non-address
bits" (not part of the virtual address) and remove them before attempting to do
a memory access on a signed pointer.

Actual behaviour: GDB does not remove these bits so it attempts to read a
completely different memory location. (one cannot be mapped in any case)

```
(gdb) p &buf
$4 = (char *(*)[16]) 0xfffffffff268
(gdb) p buf_with_non_address
$5 = 0xff75fffffffff268 <error: Cannot access memory at address
0xff75fffffffff268>
(gdb) x buf
0xfffffffff268: 0x00000000
(gdb) x buf_with_non_address
0xff75fffffffff268:     Cannot access memory at address 0xff75fffffffff268
```

I've been looking at this issue for lldb and it seems that other non-address
bits like the top byte can be left in when giving ptrace a virtual address.
Probably because the hardware ignores it for us. (this behaviour isn't
guaranteed by the kernel though as far as I know)

With pointer authentication, running code is expected to "auth" the pointer
before use. This would remove the signature bits if successful. So a memory
read/write with a signature still attached is always going to be a failure of
some kind.

GDB or GDB server should remove these signature bits before it gets to ptrace
so that the user doesn't have to manually auth these pointers.

For what it's worth, for lldb I'm looking at doing this in lldb not
lldb-server. Keeps the server simple and fixes some other things like our
memory caching along the way. Same approach might work for GDB as well.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |luis.machado at arm dot com
           Assignee|unassigned at sourceware dot org   |luis.machado at arm dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

--- Comment #1 from Luis Machado <luis.machado at arm dot com> ---
Thanks David. I'll take a look at it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-03-07
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED

--- Comment #2 from Luis Machado <luis.machado at arm dot com> ---
Doing a quick check, it seems GDB only sanitizes the top byte. If there are
non-address bits at the second-to-last byte, it won't clear those.

(gdb) set argv=0xff00fffffffff4d8
(gdb) p *argv
$3 = 0xfffffffff715 "..."

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

--- Comment #3 from Luis Machado <luis.machado at arm dot com> ---
Thinking a bit more about this, I'm not sure if GDB/debuggers should go out of
their way to remove signature bits from the pointers.

Accessing memory using a signed pointer is invalid anyway, and will result in a
fault. Having debuggers remove that information may cause confusion for a
developer that is trying to debug a PAC-related crash of some kind, as it will
not show the signature part of the pointer.

I can imagine a scenario where a pointer wasn't signed properly, but GDB will
strip the signature of that pointer and will show things as if they were
correct, when in fact they are not. Does that make sense?

Accessing memory using a tagged pointer is valid though, but debuggers need to
be cautious not to pass tagged pointers down to syscalls. GDB does this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

--- Comment #4 from Luis Machado <luis.machado at arm dot com> ---
Does DWARF have operations to sign/unsign arbitrary registers? As far as I
remember, it only deals with the return address.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[david.spickett at linaro dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

--- Comment #5 from David Spickett <david.spickett at linaro dot org> ---
TLDR: I agree that outside of literal memory read/write we should be careful
about whether the signature is shown to the user.

> Thinking a bit more about this, I'm not sure if GDB/debuggers should go out of their way to remove signature bits from the pointers.

I agree sometimes it doesn't make sense to remove it e.g. printing a symbol's
value:

(lldb) p fn_ptr
(char (*)(size_t, int)) $0 = 0x003d0000004006ac (actual=0x00000000004006ac
a.out`checked_mmap at main.c:13:48)

lldb has decided to show both if it finds a symbol using the stripped address.
So if you weren't expecting it to be signed then you'd hopefully realise here
but if you are in some PAC situation it's more convenient (this code was added
by Apple who have an ABI that uses it).

> Accessing memory using a signed pointer is invalid anyway, and will result in a fault. Having debuggers remove that information may cause confusion for a developer that is trying to debug a PAC-related crash of some kind, as it will not show the signature part of the pointer.
> I can imagine a scenario where a pointer wasn't signed properly, but GDB will strip the signature of that pointer and will show things as if they were correct, when in fact they are not. Does that make sense?

I think we agree here. If we're printing a symbol or a register or the result
of some expression - leave the bits in.

If you're giving that to some command that shows the content of memory - remove
the bits and use the virtual address.

We could tell the user that the signature is invalid, maybe by JIT-ing an auth
sequence but I think going ahead and reading the memory for them makes sense
for now.
(Also we don't know which key was used or 100% whether it's a code or data
pointer, even if the user told us what key was used. I might look into this, it
could make an interesting add on script even if it is a total hack)

> Accessing memory using a tagged pointer is valid though, but debuggers need to be cautious not to pass tagged pointers down to syscalls. GDB does this.

This is 99% of the situations I'm stripping the address in lldb. Any time
you're saying what is the value of this symbol/register, it's left intact. Only
if you try to do a memory read with it do we remove the bits. So the example
above you see the signature but if you give the symbol name to "memory read"
it'll just use the virtual address.

I guess the overall thing here is should the user be restricted during debug -
by what is allowed to happen at runtime. Given that debuggers can do things
like write to read only pages and read tagged memory with any tag they want,
the answer seems to be no. However, where it makes sense we could tell the user
that it would not work at runtime.

It's tricky. You could overflow into the upper bits of a pointer via some bad
math and because you're on a ptrauth system the debugger removes them and it
seems like it all works but fails at runtime. I don't know of a way of still
catching that whilst not making it inconvenient to work with signed pointers if
you are opting into them.
(or you could be mixing signed and unsigned pointers in the same program)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

--- Comment #6 from Luis Machado <luis.machado at arm dot com> ---
I see your point about making it a bit more convenient for the user. It's just
that we're dealing with best-effort cases here, as we don't know some of the
intentions the developer has before going and processing pointers.

It wouldn't be nice to have two distinct behaviors for the debuggers though.

As GDB already removes the (hardcoded) top 8 bits, I'm considering modifying
the hook to actually return a stripped pointer as opposed to letting generic
code remove X number of top bits.

This way we can make sure we output the proper stripped pointer. GDB had an
issue before where it failed to deal with kernel pointers, as it had to
sign-extend them while stripping the top bits.

I'll put something together along with a testcase.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |29421


Referenced Bugs:

https://sourceware.org/bugzilla/show_bug.cgi?id=29421
[Bug 29421] Extend aarch64 pauth xml for gdbstub and kernel mode
-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Luis Machado <luisgpm@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d88cb738e6a7a7179dfaff8af78d69250c852af1

commit d88cb738e6a7a7179dfaff8af78d69250c852af1
Author: Luis Machado <luis.machado@arm.com>
Date:   Tue May 24 23:31:09 2022 +0100

    [aarch64] Fix removal of non-address bits for PAuth

    PR gdb/28947

    The address_significant gdbarch setting was introduced as a way to remove
    non-address bits from pointers, and it is specified by a constant.  This
    constant represents the number of address bits in a pointer.

    Right now AArch64 is the only architecture that uses it, and 56 was a
    correct option so far.

    But if we are using Pointer Authentication (PAuth), we might use up to 2
bytes
    from the address space to store the required information.  We could also
have
    cases where we're using both PAuth and MTE.

    We could adjust the constant to 48 to cover those cases, but this doesn't
    cover the case where GDB needs to sign-extend kernel addresses after
removal
    of the non-address bits.

    This has worked so far because bit 55 is used to select between
kernel-space
    and user-space addresses.  But trying to clear a range of bits crossing the
    bit 55 boundary requires the hook to be smarter.

    The following patch renames the gdbarch hook from significant_addr_bit to
    remove_non_address_bits and passes a pointer as opposed to the number of
    bits.  The hook is now responsible for removing the required non-address
bits
    and sign-extending the address if needed.

    While at it, make GDB and GDBServer share some more code for aarch64 and
add a
    new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.

    Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947

    Approved-By: Simon Marchi <simon.marchi@efficios.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28947] GDB does not remove AArch64 pointer signatures before doing memory accesses

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28947

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Luis Machado <luis.machado at arm dot com> ---
Fixed on master.

-- 
You are receiving this mail because:
You are on the CC list for the bug.