Message ID | 20190516103655.5509-1-raphael.gault@arm.com (mailing list archive) |
---|---|
Headers | show |
Series | objtool: Add support for Arm64 | expand |
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).
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.
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.
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.
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; + } +}
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.
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,
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 :-)