mbox series

[RFC,V2,00/16] objtool: Add support for Arm64

Message ID 20190516103655.5509-1-raphael.gault@arm.com (mailing list archive)
Headers show
Series objtool: Add support for Arm64 | expand

Message

Raphael Gault May 16, 2019, 10:36 a.m. UTC
As of now, objtool only supports the x86_64 architecture but the
groundwork has already been done in order to add support for other
architectures without too much effort.

This series of patches adds support for the arm64 architecture
based on the Armv8.5 Architecture Reference Manual.

Objtool will be a valuable tool to progress and provide more guarentees
on live patching which is a work in progress for arm64.

Once we have the base of objtool working the next steps will be to
port Peter Z's uaccess validation for arm64.

RFC: In order to differentiate the different uses of the `brk`
instruction on arm64 I intended to use the
`include/generated/asm-offsets.h` header file (copying it to
tools/include/generated/asm-offsets.h). However since in is
generated later than objtool in the build process I wasn't able to do
it. I wanted to use it to have access to the info about the
`struct alt_instr` and `struct bug_entry`.

Noteworthy points:
* I still haven't figured out how to detect switch-tables on arm64. I
have a better understanding of them but still haven't implemented checks
as it doesn't look trivial at all.
* I still use the `arch_is_sibling_call` function to differentiate the
use cases of the `br` instruction on arm64. Even though I updated the
checks, it is still based on going back in the instruction stream, which
as Peter Z. pointed out is not safe. I shall work on an alternative
solution.

Changes from V2:
* Rebase on the -tip tree (which contains the latest objtool features)
* Split into more precise patches in order to highlight the changes
that were made.
* Correct patches coding style to comply with linux's style.
* Refactor some code to avoid generating a fake instruction when
decoding load/store of pairs of registers.
* Make more elegant checks for arch-dependent features (switch-tables,
special sections)
* Include some patches to add exceptions in the kernel to prevent
objtool from checking/warning in particular cases.
* Introduce a new instruction type (INSN_UNKNOWN) to handle the cases
when some data is stored inside a section marked as containing
executable instructions.


Raphael Gault (16):
  objtool: Add abstraction for computation of symbols offsets
  objtool: orc: Refactor ORC API for other architectures to implement.
  objtool: Move registers and control flow to arch-dependent code
  objtool: arm64: Add required implementation for supporting the aarch64
    architecture in objtool.
  objtool: arm64: Handle hypercalls as nops
  arm64: alternative: Mark .altinstr_replacement as containing
    executable instructions
  objtool: special: Adapt special section handling
  objtool: arm64: Adapt the stack frame checks for arm architecture
  arm64: assembler: Add macro to annotate asm function having non
    standard stack-frame.
  arm64: sleep: Prevent stack frame warnings from objtool
  objtool: arm64: Enable stack validation for arm64
  arm64: kvm: Annotate non-standard stack frame functions
  arm64: kernel: Add exception on kuser32 to prevent stack analysis
  arm64: crypto: Add exceptions for crypto object to prevent stack
    analysis
  objtool: Introduce INSN_UNKNOWN type
  arm64: kernel: Annotate non-standard stack frame functions

 arch/arm64/Kconfig                            |    1 +
 arch/arm64/crypto/Makefile                    |    3 +
 arch/arm64/include/asm/alternative.h          |    2 +-
 arch/arm64/include/asm/assembler.h            |   13 +
 arch/arm64/kernel/Makefile                    |    3 +
 arch/arm64/kernel/hyp-stub.S                  |    2 +
 arch/arm64/kernel/sleep.S                     |    4 +
 arch/arm64/kvm/hyp-init.S                     |    2 +
 arch/arm64/kvm/hyp/entry.S                    |    2 +
 tools/objtool/Build                           |    2 -
 tools/objtool/arch.h                          |   21 +-
 tools/objtool/arch/arm64/Build                |    8 +
 tools/objtool/arch/arm64/bit_operations.c     |   67 +
 tools/objtool/arch/arm64/decode.c             | 2809 +++++++++++++++++
 .../objtool/arch/arm64/include/arch_special.h |   42 +
 .../arch/arm64/include/asm/orc_types.h        |   96 +
 .../arch/arm64/include/bit_operations.h       |   24 +
 tools/objtool/arch/arm64/include/cfi.h        |   74 +
 .../objtool/arch/arm64/include/insn_decode.h  |  211 ++
 tools/objtool/arch/arm64/orc_dump.c           |   26 +
 tools/objtool/arch/arm64/orc_gen.c            |   40 +
 tools/objtool/arch/x86/Build                  |    3 +
 tools/objtool/arch/x86/decode.c               |   16 +
 tools/objtool/arch/x86/include/arch_special.h |   45 +
 tools/objtool/{ => arch/x86/include}/cfi.h    |    0
 tools/objtool/{ => arch/x86}/orc_dump.c       |    4 +-
 tools/objtool/{ => arch/x86}/orc_gen.c        |  104 +-
 tools/objtool/check.c                         |  239 +-
 tools/objtool/check.h                         |    1 +
 tools/objtool/elf.c                           |    3 +-
 tools/objtool/orc.h                           |    4 +-
 tools/objtool/special.c                       |   28 +-
 tools/objtool/special.h                       |    3 +
 33 files changed, 3753 insertions(+), 149 deletions(-)
 create mode 100644 tools/objtool/arch/arm64/Build
 create mode 100644 tools/objtool/arch/arm64/bit_operations.c
 create mode 100644 tools/objtool/arch/arm64/decode.c
 create mode 100644 tools/objtool/arch/arm64/include/arch_special.h
 create mode 100644 tools/objtool/arch/arm64/include/asm/orc_types.h
 create mode 100644 tools/objtool/arch/arm64/include/bit_operations.h
 create mode 100644 tools/objtool/arch/arm64/include/cfi.h
 create mode 100644 tools/objtool/arch/arm64/include/insn_decode.h
 create mode 100644 tools/objtool/arch/arm64/orc_dump.c
 create mode 100644 tools/objtool/arch/arm64/orc_gen.c
 create mode 100644 tools/objtool/arch/x86/include/arch_special.h
 rename tools/objtool/{ => arch/x86/include}/cfi.h (100%)
 rename tools/objtool/{ => arch/x86}/orc_dump.c (98%)
 rename tools/objtool/{ => arch/x86}/orc_gen.c (69%)

Comments

Josh Poimboeuf May 16, 2019, 2:29 p.m. UTC | #1
On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
> Noteworthy points:
> * I still haven't figured out how to detect switch-tables on arm64. I
> have a better understanding of them but still haven't implemented checks
> as it doesn't look trivial at all.

Switch tables were tricky to get right on x86.  If you share an example
(or even just a .o file) I can take a look.  Hopefully they're somewhat
similar to x86 switch tables.  Otherwise we may want to consider a
different approach (for example maybe a GCC plugin could help annotate
them).
Raphael Gault May 21, 2019, 12:50 p.m. UTC | #2
Hi Josh,

Thanks for offering your help and sorry for the late answer.

My understanding is that a table of offsets is built by GCC, those
offsets being scaled by 4 before adding them to the base label.
I believe the offsets are stored in the .rodata section. To find the
size of that table, it is needed to find a comparison, which can be
optimized out apprently. In that case the end of the array can be found
by locating labels pointing to data behind it (which is not 100% safe).

On 5/16/19 3:29 PM, Josh Poimboeuf wrote:
> On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
>> Noteworthy points:
>> * I still haven't figured out how to detect switch-tables on arm64. I
>> have a better understanding of them but still haven't implemented checks
>> as it doesn't look trivial at all.
>
> Switch tables were tricky to get right on x86.  If you share an example
> (or even just a .o file) I can take a look.  Hopefully they're somewhat
> similar to x86 switch tables.  Otherwise we may want to consider a
> different approach (for example maybe a GCC plugin could help annotate
> them).
>

The case which made me realize the issue is the one of
arch/arm64/kernel/module.o:apply_relocate_add:

```
What seems to happen in the case of module.o is:
  334:   90000015        adrp    x21, 0 <do_reloc>
which retrieves the location of an offset in the rodata section, and a
bit later we do some extra computation with it in order to compute the
jump destination:
  3e0:   78625aa0        ldrh    w0, [x21, w2, uxtw #1]
  3e4:   10000061        adr     x1, 3f0 <apply_relocate_add+0xf8>
  3e8:   8b20a820        add     x0, x1, w0, sxth #2
  3ec:   d61f0000        br      x0
```

Please keep in mind that the actual offsets might vary.

I'm happy to provide more details about what I have identified if you
want me to.

Thanks,

--
Raphael Gault
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Josh Poimboeuf May 22, 2019, 11:11 p.m. UTC | #3
On Tue, May 21, 2019 at 12:50:57PM +0000, Raphael Gault wrote:
> Hi Josh,
> 
> Thanks for offering your help and sorry for the late answer.
> 
> My understanding is that a table of offsets is built by GCC, those
> offsets being scaled by 4 before adding them to the base label.
> I believe the offsets are stored in the .rodata section. To find the
> size of that table, it is needed to find a comparison, which can be
> optimized out apprently. In that case the end of the array can be found
> by locating labels pointing to data behind it (which is not 100% safe).
> 
> On 5/16/19 3:29 PM, Josh Poimboeuf wrote:
> > On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
> >> Noteworthy points:
> >> * I still haven't figured out how to detect switch-tables on arm64. I
> >> have a better understanding of them but still haven't implemented checks
> >> as it doesn't look trivial at all.
> >
> > Switch tables were tricky to get right on x86.  If you share an example
> > (or even just a .o file) I can take a look.  Hopefully they're somewhat
> > similar to x86 switch tables.  Otherwise we may want to consider a
> > different approach (for example maybe a GCC plugin could help annotate
> > them).
> >
> 
> The case which made me realize the issue is the one of
> arch/arm64/kernel/module.o:apply_relocate_add:
> 
> ```
> What seems to happen in the case of module.o is:
>   334:   90000015        adrp    x21, 0 <do_reloc>
> which retrieves the location of an offset in the rodata section, and a
> bit later we do some extra computation with it in order to compute the
> jump destination:
>   3e0:   78625aa0        ldrh    w0, [x21, w2, uxtw #1]
>   3e4:   10000061        adr     x1, 3f0 <apply_relocate_add+0xf8>
>   3e8:   8b20a820        add     x0, x1, w0, sxth #2
>   3ec:   d61f0000        br      x0
> ```
> 
> Please keep in mind that the actual offsets might vary.
> 
> I'm happy to provide more details about what I have identified if you
> want me to.

Thanks.  I'll try to take a deeper look.

Were these patches based on tip/master?  There were some minor conflicts
in arch/arm64/Kconfig and arch/arm64/kernel/Makefile.

I'm also getting a build failure on arm64:

  make[4]: *** No rule to make target '/root/linux/tools/objtool/arch/arm64/arch_special.o', needed by '/root/linux/tools/objtool/arch/arm64/objtool-in.o'.  Stop

It looks like arch/arm64/Build and arch/x86/Build are trying to build
from arch_special.c which doesn't exist.
Raphael Gault May 23, 2019, 7:31 a.m. UTC | #4
Hi Josh,

On 5/23/19 12:11 AM, Josh Poimboeuf wrote:
> On Tue, May 21, 2019 at 12:50:57PM +0000, Raphael Gault wrote:
>> Hi Josh,
>>
>> Thanks for offering your help and sorry for the late answer.
>>
>> My understanding is that a table of offsets is built by GCC, those
>> offsets being scaled by 4 before adding them to the base label.
>> I believe the offsets are stored in the .rodata section. To find the
>> size of that table, it is needed to find a comparison, which can be
>> optimized out apprently. In that case the end of the array can be found
>> by locating labels pointing to data behind it (which is not 100% safe).
>>
>> On 5/16/19 3:29 PM, Josh Poimboeuf wrote:
>>> On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
>>>> Noteworthy points:
>>>> * I still haven't figured out how to detect switch-tables on arm64. I
>>>> have a better understanding of them but still haven't implemented checks
>>>> as it doesn't look trivial at all.
>>>
>>> Switch tables were tricky to get right on x86.  If you share an example
>>> (or even just a .o file) I can take a look.  Hopefully they're somewhat
>>> similar to x86 switch tables.  Otherwise we may want to consider a
>>> different approach (for example maybe a GCC plugin could help annotate
>>> them).
>>>
>>
>> The case which made me realize the issue is the one of
>> arch/arm64/kernel/module.o:apply_relocate_add:
>>
>> ```
>> What seems to happen in the case of module.o is:
>>    334:   90000015        adrp    x21, 0 <do_reloc>
>> which retrieves the location of an offset in the rodata section, and a
>> bit later we do some extra computation with it in order to compute the
>> jump destination:
>>    3e0:   78625aa0        ldrh    w0, [x21, w2, uxtw #1]
>>    3e4:   10000061        adr     x1, 3f0 <apply_relocate_add+0xf8>
>>    3e8:   8b20a820        add     x0, x1, w0, sxth #2
>>    3ec:   d61f0000        br      x0
>> ```
>>
>> Please keep in mind that the actual offsets might vary.
>>
>> I'm happy to provide more details about what I have identified if you
>> want me to.
>
> Thanks.  I'll try to take a deeper look.
>
> Were these patches based on tip/master?  There were some minor conflicts
> in arch/arm64/Kconfig and arch/arm64/kernel/Makefile.
>

These patches were based on tip/core-objtool. I'll base the next version
on master. Sorry about that.

> I'm also getting a build failure on arm64:
>
>    make[4]: *** No rule to make target '/root/linux/tools/objtool/arch/arm64/arch_special.o', needed by '/root/linux/tools/objtool/arch/arm64/objtool-in.o'.  Stop
>
> It looks like arch/arm64/Build and arch/x86/Build are trying to build
> from arch_special.c which doesn't exist.
>

Indeed, I apologize deeply for this embarrassing mistake which will be
fixed very soon.

Thanks,

--
Raphael Gault
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Raphael Gault May 23, 2019, 7:59 a.m. UTC | #5
Hi Josh,

On 5/23/19 12:11 AM, Josh Poimboeuf wrote:
[...]
> I'm also getting a build failure on arm64:
>
>    make[4]: *** No rule to make target '/root/linux/tools/objtool/arch/arm64/arch_special.o', needed by '/root/linux/tools/objtool/arch/arm64/objtool-in.o'.  Stop
>
> It looks like arch/arm64/Build and arch/x86/Build are trying to build
> from arch_special.c which doesn't exist.
>

Please find attach a corrective patch. I am sorry about that. I will
integrate this fix correctly in my next version.

Thanks,

--
Raphael Gault
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
From 10ad134841c36ffc60078a9f5a0d69bcebbff175 Mon Sep 17 00:00:00 2001
From: Raphael Gault <raphael.gault@arm.com>
Date: Thu, 23 May 2019 08:56:37 +0100
Subject: [PATCH] objtool: special: Correct mistake missing file

Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
 tools/objtool/arch/arm64/arch_special.c | 22 +++++++++++++++++++
 tools/objtool/arch/x86/arch_special.c   | 28 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 tools/objtool/arch/arm64/arch_special.c
 create mode 100644 tools/objtool/arch/x86/arch_special.c

diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
new file mode 100644
index 000000000000..a21d28876317
--- /dev/null
+++ b/tools/objtool/arch/arm64/arch_special.c
@@ -0,0 +1,22 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../../special.h"
+#include "arch_special.h"
+
+void arch_force_alt_path(unsigned short feature,
+			 bool uaccess,
+			 struct special_alt *alt)
+{
+}
diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
new file mode 100644
index 000000000000..6583a1770bb2
--- /dev/null
+++ b/tools/objtool/arch/x86/arch_special.c
@@ -0,0 +1,28 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../../special.h"
+#include "arch_special.h"
+
+void arch_force_alt_path(unsigned short feature,
+			 bool uaccess,
+			 struct special_alt *alt)
+{
+		if (feature == X86_FEATURE_SMAP) {
+			if (uaccess)
+				alt->skip_orig = true;
+			else
+				alt->skip_alt = true;
+		}
+}
Josh Poimboeuf May 28, 2019, 10:24 p.m. UTC | #6
On Tue, May 21, 2019 at 12:50:57PM +0000, Raphael Gault wrote:
> Hi Josh,
> 
> Thanks for offering your help and sorry for the late answer.
> 
> My understanding is that a table of offsets is built by GCC, those
> offsets being scaled by 4 before adding them to the base label.
> I believe the offsets are stored in the .rodata section. To find the
> size of that table, it is needed to find a comparison, which can be
> optimized out apprently. In that case the end of the array can be found
> by locating labels pointing to data behind it (which is not 100% safe).
> 
> On 5/16/19 3:29 PM, Josh Poimboeuf wrote:
> > On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
> >> Noteworthy points:
> >> * I still haven't figured out how to detect switch-tables on arm64. I
> >> have a better understanding of them but still haven't implemented checks
> >> as it doesn't look trivial at all.
> >
> > Switch tables were tricky to get right on x86.  If you share an example
> > (or even just a .o file) I can take a look.  Hopefully they're somewhat
> > similar to x86 switch tables.  Otherwise we may want to consider a
> > different approach (for example maybe a GCC plugin could help annotate
> > them).
> >
> 
> The case which made me realize the issue is the one of
> arch/arm64/kernel/module.o:apply_relocate_add:
> 
> ```
> What seems to happen in the case of module.o is:
>   334:   90000015        adrp    x21, 0 <do_reloc>
> which retrieves the location of an offset in the rodata section, and a
> bit later we do some extra computation with it in order to compute the
> jump destination:
>   3e0:   78625aa0        ldrh    w0, [x21, w2, uxtw #1]
>   3e4:   10000061        adr     x1, 3f0 <apply_relocate_add+0xf8>
>   3e8:   8b20a820        add     x0, x1, w0, sxth #2
>   3ec:   d61f0000        br      x0
> ```
> 
> Please keep in mind that the actual offsets might vary.
> 
> I'm happy to provide more details about what I have identified if you
> want me to.

I get the feeling this is going to be trickier than x86 switch tables
(which have already been tricky enough).

On x86, there's a .rela.rodata section which applies relocations to
.rodata.  The presence of those relocations makes it relatively easy to
differentiate switch tables from other read-only data.  For example, we
can tell that a switch table ends when either a) there's not a text
relocation or b) another switch table begins.

But with arm64 I don't see a deterministic way to do that, because the
table offsets are hard-coded in .rodata, with no relocations.

From talking with Kamalesh I got the impression that we might have a
similar issue for powerpc.

So I'm beginning to think we'll need compiler help.  Like a GCC plugin
that annotates at least the following switch table metadata:

- Branch instruction address
- Switch table address
- Switch table entry size
- Switch table size

The GCC plugin could write all the above metadata into a special section
which gets discarded at link time.  I can look at implementing it,
though I'll be traveling for two out of the next three weeks so it may
be a while before I can get to it.
Raphael Gault June 13, 2019, 3:55 p.m. UTC | #7
Hi Josh,

On 5/28/19 11:24 PM, Josh Poimboeuf wrote:
> On Tue, May 21, 2019 at 12:50:57PM +0000, Raphael Gault wrote:
>> Hi Josh,
>>
>> Thanks for offering your help and sorry for the late answer.
>>
>> My understanding is that a table of offsets is built by GCC, those
>> offsets being scaled by 4 before adding them to the base label.
>> I believe the offsets are stored in the .rodata section. To find the
>> size of that table, it is needed to find a comparison, which can be
>> optimized out apprently. In that case the end of the array can be found
>> by locating labels pointing to data behind it (which is not 100% safe).
>>
>> On 5/16/19 3:29 PM, Josh Poimboeuf wrote:
>>> On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
>>>> Noteworthy points:
>>>> * I still haven't figured out how to detect switch-tables on arm64. I
>>>> have a better understanding of them but still haven't implemented checks
>>>> as it doesn't look trivial at all.
>>>
>>> Switch tables were tricky to get right on x86.  If you share an example
>>> (or even just a .o file) I can take a look.  Hopefully they're somewhat
>>> similar to x86 switch tables.  Otherwise we may want to consider a
>>> different approach (for example maybe a GCC plugin could help annotate
>>> them).
>>>
>>
>> The case which made me realize the issue is the one of
>> arch/arm64/kernel/module.o:apply_relocate_add:
>>
>> ```
>> What seems to happen in the case of module.o is:
>>    334:   90000015        adrp    x21, 0 <do_reloc>
>> which retrieves the location of an offset in the rodata section, and a
>> bit later we do some extra computation with it in order to compute the
>> jump destination:
>>    3e0:   78625aa0        ldrh    w0, [x21, w2, uxtw #1]
>>    3e4:   10000061        adr     x1, 3f0 <apply_relocate_add+0xf8>
>>    3e8:   8b20a820        add     x0, x1, w0, sxth #2
>>    3ec:   d61f0000        br      x0
>> ```
>>
>> Please keep in mind that the actual offsets might vary.
>>
>> I'm happy to provide more details about what I have identified if you
>> want me to.
> 
> I get the feeling this is going to be trickier than x86 switch tables
> (which have already been tricky enough).
> 
> On x86, there's a .rela.rodata section which applies relocations to
> .rodata.  The presence of those relocations makes it relatively easy to
> differentiate switch tables from other read-only data.  For example, we
> can tell that a switch table ends when either a) there's not a text
> relocation or b) another switch table begins.
> 
> But with arm64 I don't see a deterministic way to do that, because the
> table offsets are hard-coded in .rodata, with no relocations.
> 
>  From talking with Kamalesh I got the impression that we might have a
> similar issue for powerpc.
> 
> So I'm beginning to think we'll need compiler help.  Like a GCC plugin
> that annotates at least the following switch table metadata:
> 
> - Branch instruction address
> - Switch table address
> - Switch table entry size
> - Switch table size
> 
> The GCC plugin could write all the above metadata into a special section
> which gets discarded at link time.  I can look at implementing it,
> though I'll be traveling for two out of the next three weeks so it may
> be a while before I can get to it.
> 

I am completely new to GCC plugins but I had a look and I think I found 
a possible solution to retrieve at least part of this information using 
the RTL representation in GCC. I can't say it will work for sure but I 
would be happy to discuss it with you if you want.
Although there are still some area I need to investigate related to 
interacting with the RTL representation and storing info into the ELF
I'd be interested in giving it a try, if you are okay with that.

Thanks,
Josh Poimboeuf June 13, 2019, 4:09 p.m. UTC | #8
On Thu, Jun 13, 2019 at 04:55:31PM +0100, Raphael Gault wrote:
> Hi Josh,
> 
> On 5/28/19 11:24 PM, Josh Poimboeuf wrote:
> > On Tue, May 21, 2019 at 12:50:57PM +0000, Raphael Gault wrote:
> > > Hi Josh,
> > > 
> > > Thanks for offering your help and sorry for the late answer.
> > > 
> > > My understanding is that a table of offsets is built by GCC, those
> > > offsets being scaled by 4 before adding them to the base label.
> > > I believe the offsets are stored in the .rodata section. To find the
> > > size of that table, it is needed to find a comparison, which can be
> > > optimized out apprently. In that case the end of the array can be found
> > > by locating labels pointing to data behind it (which is not 100% safe).
> > > 
> > > On 5/16/19 3:29 PM, Josh Poimboeuf wrote:
> > > > On Thu, May 16, 2019 at 11:36:39AM +0100, Raphael Gault wrote:
> > > > > Noteworthy points:
> > > > > * I still haven't figured out how to detect switch-tables on arm64. I
> > > > > have a better understanding of them but still haven't implemented checks
> > > > > as it doesn't look trivial at all.
> > > > 
> > > > Switch tables were tricky to get right on x86.  If you share an example
> > > > (or even just a .o file) I can take a look.  Hopefully they're somewhat
> > > > similar to x86 switch tables.  Otherwise we may want to consider a
> > > > different approach (for example maybe a GCC plugin could help annotate
> > > > them).
> > > > 
> > > 
> > > The case which made me realize the issue is the one of
> > > arch/arm64/kernel/module.o:apply_relocate_add:
> > > 
> > > ```
> > > What seems to happen in the case of module.o is:
> > >    334:   90000015        adrp    x21, 0 <do_reloc>
> > > which retrieves the location of an offset in the rodata section, and a
> > > bit later we do some extra computation with it in order to compute the
> > > jump destination:
> > >    3e0:   78625aa0        ldrh    w0, [x21, w2, uxtw #1]
> > >    3e4:   10000061        adr     x1, 3f0 <apply_relocate_add+0xf8>
> > >    3e8:   8b20a820        add     x0, x1, w0, sxth #2
> > >    3ec:   d61f0000        br      x0
> > > ```
> > > 
> > > Please keep in mind that the actual offsets might vary.
> > > 
> > > I'm happy to provide more details about what I have identified if you
> > > want me to.
> > 
> > I get the feeling this is going to be trickier than x86 switch tables
> > (which have already been tricky enough).
> > 
> > On x86, there's a .rela.rodata section which applies relocations to
> > .rodata.  The presence of those relocations makes it relatively easy to
> > differentiate switch tables from other read-only data.  For example, we
> > can tell that a switch table ends when either a) there's not a text
> > relocation or b) another switch table begins.
> > 
> > But with arm64 I don't see a deterministic way to do that, because the
> > table offsets are hard-coded in .rodata, with no relocations.
> > 
> >  From talking with Kamalesh I got the impression that we might have a
> > similar issue for powerpc.
> > 
> > So I'm beginning to think we'll need compiler help.  Like a GCC plugin
> > that annotates at least the following switch table metadata:
> > 
> > - Branch instruction address
> > - Switch table address
> > - Switch table entry size
> > - Switch table size
> > 
> > The GCC plugin could write all the above metadata into a special section
> > which gets discarded at link time.  I can look at implementing it,
> > though I'll be traveling for two out of the next three weeks so it may
> > be a while before I can get to it.
> > 
> 
> I am completely new to GCC plugins but I had a look and I think I found a
> possible solution to retrieve at least part of this information using the
> RTL representation in GCC. I can't say it will work for sure but I would be
> happy to discuss it with you if you want.
> Although there are still some area I need to investigate related to
> interacting with the RTL representation and storing info into the ELF
> I'd be interested in giving it a try, if you are okay with that.

Sounds promising.  I've been stretched thin lately with other work, and
I'll be out again next week, so please go ahead :-)