diff mbox

[REPOST] ARM: Thumb-2: Relax relocation requirements for non-function symbols

Message ID 1306859269-21304-1-git-send-email-dave.martin@linaro.org
State Accepted
Headers show

Commit Message

Dave Martin May 31, 2011, 4:27 p.m. UTC
The "Thumb bit" of a symbol is only really meaningful for function
symbols (STT_FUNC).

However, sometimes a branch is relocated against a non-function
symbol; for example, PC-relative branches to anonymous assembler
local symbols are typically fixed up against the start-of-section
symbol, which is not a function symbol.  Some inline assembler
generates references of this type, such as fixup code generated by
macros in <asm/uaccess.h>.

The existing relocation code for R_ARM_THM_CALL/R_ARM_THM_JUMP24
interprets this case as an error, because the target symbol appears
to be an ARM symbol; but this is really not the case, since the
target symbol is just a base in these cases.  The addend defines
the precise offset to the target location, but since the addend is
encoded in a non-interworking Thumb branch instruction, there is no
explicit Thumb bit in the addend.  Because these instructions never
interwork, the implied Thumb bit in the addend is 1, and the
destination is Thumb by definition.

This patch removes the extraneous Thumb bit check for non-function
symbols, enabling modules containing the affected relocation types
to be loaded.  No modification to the actual relocation code is
required, since this code does not take bit[0] of the
location->destination offset into account in any case.

Function symbols are always checked for interworking conflicts, as
before.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---

[Reposting to give people a chance to comment.]

 arch/arm/kernel/module.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

Comments

Catalin Marinas June 1, 2011, 9:27 a.m. UTC | #1
On 31 May 2011 17:27, Dave Martin <dave.martin@linaro.org> wrote:
> The "Thumb bit" of a symbol is only really meaningful for function
> symbols (STT_FUNC).
>
> However, sometimes a branch is relocated against a non-function
> symbol; for example, PC-relative branches to anonymous assembler
> local symbols are typically fixed up against the start-of-section
> symbol, which is not a function symbol.  Some inline assembler
> generates references of this type, such as fixup code generated by
> macros in <asm/uaccess.h>.
>
> The existing relocation code for R_ARM_THM_CALL/R_ARM_THM_JUMP24
> interprets this case as an error, because the target symbol appears
> to be an ARM symbol; but this is really not the case, since the
> target symbol is just a base in these cases.  The addend defines
> the precise offset to the target location, but since the addend is
> encoded in a non-interworking Thumb branch instruction, there is no
> explicit Thumb bit in the addend.  Because these instructions never
> interwork, the implied Thumb bit in the addend is 1, and the
> destination is Thumb by definition.
>
> This patch removes the extraneous Thumb bit check for non-function
> symbols, enabling modules containing the affected relocation types
> to be loaded.  No modification to the actual relocation code is
> required, since this code does not take bit[0] of the
> location->destination offset into account in any case.
>
> Function symbols are always checked for interworking conflicts, as
> before.
>
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Dave Martin June 1, 2011, 10:48 a.m. UTC | #2
On Wed, Jun 01, 2011 at 10:27:14AM +0100, Catalin Marinas wrote:
> On 31 May 2011 17:27, Dave Martin <dave.martin@linaro.org> wrote:
> > The "Thumb bit" of a symbol is only really meaningful for function
> > symbols (STT_FUNC).
> >
> > However, sometimes a branch is relocated against a non-function
> > symbol; for example, PC-relative branches to anonymous assembler
> > local symbols are typically fixed up against the start-of-section
> > symbol, which is not a function symbol.  Some inline assembler
> > generates references of this type, such as fixup code generated by
> > macros in <asm/uaccess.h>.
> >
> > The existing relocation code for R_ARM_THM_CALL/R_ARM_THM_JUMP24
> > interprets this case as an error, because the target symbol appears
> > to be an ARM symbol; but this is really not the case, since the
> > target symbol is just a base in these cases.  The addend defines
> > the precise offset to the target location, but since the addend is
> > encoded in a non-interworking Thumb branch instruction, there is no
> > explicit Thumb bit in the addend.  Because these instructions never
> > interwork, the implied Thumb bit in the addend is 1, and the
> > destination is Thumb by definition.
> >
> > This patch removes the extraneous Thumb bit check for non-function
> > symbols, enabling modules containing the affected relocation types
> > to be loaded.  No modification to the actual relocation code is
> > required, since this code does not take bit[0] of the
> > location->destination offset into account in any case.
> >
> > Function symbols are always checked for interworking conflicts, as
> > before.
> >
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks
---Dave
diff mbox

Patch

diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index fee7c36..016d6a0 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -193,8 +193,17 @@  apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
 				offset -= 0x02000000;
 			offset += sym->st_value - loc;
 
-			/* only Thumb addresses allowed (no interworking) */
-			if (!(offset & 1) ||
+			/*
+			 * For function symbols, only Thumb addresses are
+			 * allowed (no interworking).
+			 *
+			 * For non-function symbols, the destination
+			 * has no specific ARM/Thumb disposition, so
+			 * the branch is resolved under the assumption
+			 * that interworking is not required.
+			 */
+			if ((ELF32_ST_TYPE(sym->st_info) == STT_FUNC &&
+				!(offset & 1)) ||
 			    offset <= (s32)0xff000000 ||
 			    offset >= (s32)0x01000000) {
 				pr_err("%s: section %u reloc %u sym '%s': relocation %u out of range (%#lx -> %#x)\n",