From 779635aec2c6446ab8cd8b9ee62481265c486d18 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Mon, 18 Jul 2022 20:26:34 -0400 Subject: [PATCH] GP-2330 Correct ELF symbol import issue with SHN_ABS and general cleanup of symbol address determination --- .../app/util/bin/format/elf/ElfHeader.java | 2 +- .../format/elf/ElfSectionHeaderConstants.java | 6 +- .../app/util/bin/format/elf/ElfSymbol.java | 24 +-- .../bin/format/elf/extend/ElfLoadAdapter.java | 2 +- .../app/util/opinion/ElfProgramBuilder.java | 140 +++++++++--------- 5 files changed, 89 insertions(+), 85 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfHeader.java index e34edbdb8a..15e1a6bd1b 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfHeader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfHeader.java @@ -1427,7 +1427,7 @@ public class ElfHeader implements StructConverter, Writeable { long fileRangeLength) { long maxOffset = fileOffset + fileRangeLength - 1; for (ElfSectionHeader section : sectionHeaders) { - if (section.getType() == ElfSectionHeaderConstants.SHN_UNDEF) { + if (section.getType() == ElfSectionHeaderConstants.SHT_NULL) { continue; } long size = section.getSize(); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSectionHeaderConstants.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSectionHeaderConstants.java index a1232215b8..8e6339af80 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSectionHeaderConstants.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSectionHeaderConstants.java @@ -169,7 +169,7 @@ public class ElfSectionHeaderConstants { /**Processor-specific*/ public static final int SHF_MASKPROC = 0xf0000000; - // + // Special section index values (stored as 16-bit value) /**undefined, missing, irrelevant section*/ public static final short SHN_UNDEF = (short) 0x0000; @@ -198,7 +198,7 @@ public class ElfSectionHeaderConstants { * specific value in the range SHN_LOPROC..SHN_HIPROC, else false */ public static boolean isProcessorSpecificSymbolSectionIndex(short symbolSectionIndex) { - return symbolSectionIndex >= ElfSectionHeaderConstants.SHN_LOPROC && - symbolSectionIndex <= ElfSectionHeaderConstants.SHN_HIPROC; + return (Short.compareUnsigned(symbolSectionIndex, SHN_LOPROC) >= 0) && + (Short.compareUnsigned(symbolSectionIndex, SHN_HIPROC) <= 0); } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java index 16a0468b89..f00755c605 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java @@ -204,13 +204,11 @@ public class ElfSymbol implements ByteArrayConverter { if (st_name == 0) { if (getType() == STT_SECTION) { ElfSectionHeader[] sections = header.getSections(); - if (st_shndx < 0 || st_shndx >= sections.length) { - //invalid section reference... - //this is a bug in objcopy, whereby sections are removed - //but the corresponding section symbols are left behind. - } - else { - ElfSectionHeader section = sections[st_shndx]; + // FIXME: handle extended section indexing + int uSectionIndex = Short.toUnsignedInt(st_shndx); + if (Short.compareUnsigned(st_shndx, ElfSectionHeaderConstants.SHN_LORESERVE) < 0 && + uSectionIndex < sections.length) { + ElfSectionHeader section = sections[uSectionIndex]; nameAsString = section.getNameAsString(); } } @@ -358,7 +356,7 @@ public class ElfSymbol implements ByteArrayConverter { public boolean isExternal() { return (isGlobal() || isWeak()) && getValue() == 0 && getSize() == 0 && getType() == STT_NOTYPE && - getSectionHeaderIndex() == ElfSectionHeaderConstants.SHT_NULL; + getSectionHeaderIndex() == ElfSectionHeaderConstants.SHN_UNDEF; } /** @@ -486,6 +484,7 @@ public class ElfSymbol implements ByteArrayConverter { /** * Every symbol table entry is "defined" in relation to some section; * this member holds the relevant section header table index. + * NOTE: This value reflects the raw st_shndx value and not the extended section index value * @return the relevant section header table index */ public short getSectionHeaderIndex() { @@ -517,10 +516,11 @@ public class ElfSymbol implements ByteArrayConverter { */ @Override public String toString() { - return nameAsString + " - " + "st_value:" + Long.toHexString(st_value) + " - " + - "st_size: " + Long.toHexString(st_size) + " - " + "st_info: " + - Integer.toHexString(st_info) + " - " + "st_other: " + Integer.toHexString(st_other) + - " - " + "st_shndx:" + Integer.toHexString(st_shndx); + return nameAsString + " - " + "st_value: 0x" + Long.toHexString(st_value) + " - " + + "st_size: 0x" + Long.toHexString(st_size) + " - " + "st_info: 0x" + + Integer.toHexString(Byte.toUnsignedInt(st_info)) + " - " + "st_other: 0x" + + Integer.toHexString(Byte.toUnsignedInt(st_other)) + + " - " + "st_shndx: 0x" + Integer.toHexString(Short.toUnsignedInt(st_shndx)); } /** diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/extend/ElfLoadAdapter.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/extend/ElfLoadAdapter.java index 36f3ebf258..0be90e5ccb 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/extend/ElfLoadAdapter.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/extend/ElfLoadAdapter.java @@ -341,7 +341,7 @@ public class ElfLoadAdapter { * adjust the address and/or apply context to the intended symbol location. * @param elfLoadHelper load helper object * @param elfSymbol elf symbol - * @param address program memory address where symbol will be created + * @param address program memory address where symbol will be created. * @param isExternal true if symbol treated as external to the program and has been * assigned a fake memory address in the EXTERNAL memory block. * @return adjusted symbol address or null if extension will handle applying the elfSymbol diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfProgramBuilder.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfProgramBuilder.java index 8d08e42f6a..c2205d3748 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfProgramBuilder.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfProgramBuilder.java @@ -1394,7 +1394,7 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper { usingFakeExternal = true; } - if (elfSymbol.isObject()) { + if (elfSymbol.isObject() && address.isMemoryAddress()) { long size = elfSymbol.getSize(); if (size > 0 && size < Integer.MAX_VALUE) { dataAllocationMap.put(address, (int) size); @@ -1453,30 +1453,8 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper { AddressSpace symbolSpace = defaultSpace; long symOffset = elfSymbol.getValue(); - if (sectionIndex > 0) { - if (sectionIndex < elfSections.length) { - ElfSectionHeader symSection = elf.getSections()[sectionIndex]; - symSectionBase = findLoadAddress(symSection, 0); - if (symSectionBase == null) { - log("Unable to place symbol due to non-loaded section: " + - elfSymbol.getNameAsString() + " - value=0x" + - Long.toHexString(elfSymbol.getValue()) + ", section=" + - symSection.getNameAsString()); - return null; - } - symbolSpace = symSectionBase.getAddressSpace(); - } // else assume sections have been stripped - AddressSpace space = symbolSpace.getPhysicalSpace(); - symOffset = loadAdapter.getAdjustedMemoryOffset(symOffset, space); - if (space == defaultSpace) { - symOffset = - elf.adjustAddressForPrelink(symOffset) + getImageBaseWordAdjustmentOffset(); - } - else if (space == defaultDataSpace) { - symOffset += getImageDataBase(); - } - } - else if (sectionIndex == ElfSectionHeaderConstants.SHN_UNDEF) { // Not section relative 0x0000 (e.g., no sections defined) + boolean isAllocatedToSection = false; + if (sectionIndex == ElfSectionHeaderConstants.SHN_UNDEF) { // Not section relative 0x0000 (e.g., no sections defined) Address regAddr = findMemoryRegister(elfSymbol); if (regAddr != null) { @@ -1489,39 +1467,82 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper { symOffset = loadAdapter.getAdjustedMemoryOffset(symOffset, defaultSpace); symOffset += getImageBaseWordAdjustmentOffset(); } - else if (sectionIndex == ElfSectionHeaderConstants.SHN_ABS) { // Absolute value/address - 0xfff1 - // TODO: Which space ? Can't distinguish simple constant vs. data vs. code/default space - // The should potentially be assign a constant address instead (not possible currently) + else if (Short.compareUnsigned(sectionIndex, ElfSectionHeaderConstants.SHN_LORESERVE) < 0) { + isAllocatedToSection = true; + int uSectionIndex = Short.toUnsignedInt(sectionIndex); + if (uSectionIndex < elfSections.length) { - // Note: Assume data space - symbols will be "pinned" + ElfSectionHeader symSection = elf.getSections()[uSectionIndex]; + symSectionBase = findLoadAddress(symSection, 0); + if (symSectionBase == null) { + log("Unable to place symbol due to non-loaded section: " + + elfSymbol.getNameAsString() + " - value=0x" + + Long.toHexString(elfSymbol.getValue()) + ", section=" + + symSection.getNameAsString()); + return null; + } - // TODO: it may be inappropriate to adjust since value may not actually be a memory address - what to do? - // symOffset = loadAdapter.adjustMemoryOffset(symOffset, space); + symbolSpace = symSectionBase.getAddressSpace(); - Address regAddr = findMemoryRegister(elfSymbol); - if (regAddr != null) { - return regAddr; + if (elf.isRelocatable()) { + // Section relative symbol - ensure that symbol remains in + // overlay space even if beyond bounds of associated block + // Note: don't use symOffset variable since it may have been + // adjusted for image base + return symSectionBase.addWrapSpace(elfSymbol.getValue() * + symSectionBase.getAddressSpace().getAddressableUnitSize()); + } } - symbolSpace = getConstantSpace(); + // Unable to place symbol within relocatable if section missing/stripped + else if (elf.isRelocatable()) { + log("No Memory for symbol: " + elfSymbol.getNameAsString() + + " - 0x" + Long.toHexString(elfSymbol.getValue())); + return null; + } + + AddressSpace space = symbolSpace.getPhysicalSpace(); + symOffset = loadAdapter.getAdjustedMemoryOffset(symOffset, space); + if (space == defaultSpace) { + symOffset = + elf.adjustAddressForPrelink(symOffset) + getImageBaseWordAdjustmentOffset(); + } + else if (space == defaultDataSpace) { + symOffset += getImageDataBase(); + } + } + else if (sectionIndex == ElfSectionHeaderConstants.SHN_ABS) { // Absolute value/address - 0xfff1 + + byte type = elfSymbol.getType(); + if (type == ElfSymbol.STT_FILE) { + return null; // ignore file symbol + } + + // Absolute symbols will be pinned to associated address + symbolSpace = defaultDataSpace; + if (elfSymbol.isFunction()) { + symbolSpace = defaultSpace; + } + else { + Address regAddr = findMemoryRegister(elfSymbol); + if (regAddr != null) { + return regAddr; + } + } } else if (sectionIndex == ElfSectionHeaderConstants.SHN_COMMON) { // Common symbols - 0xfff2 ( - // TODO: Which space ? Can't distinguish data vs. code/default space - // I believe COMMON symbols must be allocated based upon their size. These symbols - // during the linking phase will generally be placed into a data section (e.g., .data, .bss) - + return Address.NO_ADDRESS; // assume unallocated/external } else { // TODO: Identify which cases if any that this is valid // SHN_LORESERVE 0xff00 // SHN_LOPROC 0xff00 // SHN_HIPROC 0xff1f - // SHN_COMMON 0xfff2 // SHN_HIRESERVE 0xffff log("Unable to place symbol: " + elfSymbol.getNameAsString() + " - value=0x" + Long.toHexString(elfSymbol.getValue()) + ", section-index=0x" + - Integer.toHexString(sectionIndex & 0xffff)); + Integer.toHexString(Short.toUnsignedInt(sectionIndex))); return null; } @@ -1531,35 +1552,15 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper { address = symbolSpace.getAddressInThisSpaceOnly(address.getOffset()); } - if (elfSymbol.isAbsolute()) { - // TODO: Many absolute values do not refer to memory at all - // should we exclude certain absolute symbols (e.g., 0, 1)? + if (isAllocatedToSection || elfSymbol.isAbsolute()) { + return address; + } - //we will just use the symbols preferred address... - } - else if (elfSymbol.isExternal() || elfSymbol.isCommon()) { + // Identify special cases which should be treated as external (return NO_ADDRESS) + + if (elfSymbol.isExternal()) { return Address.NO_ADDRESS; } - else if (elf.isRelocatable()) { - if (sectionIndex < 0 || sectionIndex >= elfSections.length) { - log("Error creating symbol: " + elfSymbol.getNameAsString() + - " - 0x" + Long.toHexString(elfSymbol.getValue())); - return Address.NO_ADDRESS; - } - else if (symSectionBase == null) { - log("No Memory for symbol: " + elfSymbol.getNameAsString() + - " - 0x" + Long.toHexString(elfSymbol.getValue())); - return Address.NO_ADDRESS; - } - else { - // Section relative symbol - ensure that symbol remains in - // overlay space even if beyond bounds of associated block - // Note: don't use symOffset variable since it may have been - // adjusted for image base - address = symSectionBase.addWrapSpace(elfSymbol.getValue() * - symSectionBase.getAddressSpace().getAddressableUnitSize()); - } - } else if (!elfSymbol.isSection() && elfSymbol.getValue() == 0) { return Address.NO_ADDRESS; } @@ -1742,7 +1743,10 @@ class ElfProgramBuilder extends MemorySectionResolver implements ElfLoadHelper { throws InvalidInputException { // allow extension to either modify symbol address or fully handle it - address = elf.getLoadAdapter().evaluateElfSymbol(this, elfSymbol, address, isFakeExternal); + if (address.isMemoryAddress()) { + address = + elf.getLoadAdapter().evaluateElfSymbol(this, elfSymbol, address, isFakeExternal); + } if (address != null) { // Remember where in memory Elf symbols have been mapped