Reviewed-by: Liming Gao <[email protected]> -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Ard Biesheuvel Sent: Monday, December 07, 2015 6:08 PM To: [email protected]; Cohen, Eugene Cc: Ard Biesheuvel; Laszlo Ersek; Leif Lindholm; Gao, Liming Subject: Re: [edk2] [PATCH v3 2/3] BaseTools/GenFw RVCT: fix relocation processing of PT_DYNAMIC sections
On 2 December 2015 at 17:06, Ard Biesheuvel <[email protected]> wrote: > Unlike GNU ld, which can be instructed to emit symbol based static > relocations into fully linked binaries using the --emit-relocs command > line switch, the RVCT armlink tool can only emit dynamic relocations > into the PT_DYNAMIC segment. > > This has two consequences > . we can only identify absolute relocations, so there is no way to fix > up relative relocations between sections, or check their validity in > the PE/COFF layout > . the r_offset fields of the PT_DYNAMIC DT_REL entries are relative > either to the base of the image or to any of its segments but *not* to > the base of the input section that contains the location they refer > to, and converting them to PE/COFF image offsets is non-trivial unless > the sections are laid out in the same way in the ELF and PE/COFF > versions of the binary. > > There is really only one way to deal with this, and that is to require > that the ELF and PE/COFF versions of the binary are identical in memory. > So enforce that in the code. > > Also, fix the utterly broken relocation fixup code that dereferences > ELF32_R_SYM(r_info) both as a 1-based program header index and a > 0-based section header index. If this code ever produced working > binaries, it was purely by chance. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > BaseTools/Source/C/GenFw/Elf32Convert.c | 36 ++++++++++++++------ > 1 file changed, 26 insertions(+), 10 deletions(-) > @Liming, Yonghong If there are no objections to this patch (if affects EM_ARM only), I would like to proceed and apply it. Thanks, Ard. > diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c > b/BaseTools/Source/C/GenFw/Elf32Convert.c > index 469394540e6a..eede64576940 100644 > --- a/BaseTools/Source/C/GenFw/Elf32Convert.c > +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c > @@ -804,9 +804,7 @@ WriteRelocations32 ( > UINTN RelSize; > UINTN RelOffset; > UINTN K; > - UINT8 *Targ; > Elf32_Phdr *DynamicSegment; > - Elf32_Phdr *TargetSegment; > > for (Index = 0, FoundRelocations = FALSE; Index < mEhdr->e_shnum; Index++) > { > Elf_Shdr *RelShdr = GetShdrByIndex(Index); @@ -957,6 +955,31 @@ > WriteRelocations32 ( > Error (NULL, 0, 3000, "Invalid", "%s bad ARM dynamic > relocations.", mInImageName); > } > > + for (Index = 0; Index < mEhdr->e_shnum; Index++) { > + Elf_Shdr *shdr = GetShdrByIndex(Index); > + > + // > + // The PT_DYNAMIC section contains DT_REL relocations whose > r_offset > + // field is relative to the base of a segment (or the entire > image), > + // and not to the base of an ELF input section as is the case for > + // SHT_REL sections. This means that we cannot fix up such > relocations > + // unless we cross-reference ELF sections and segments, considering > + // that the output placement recorded in mCoffSectionsOffset[] is > + // section based, not segment based. > + // > + // Fortunately, there is a simple way around this: we require that > the > + // in-memory layout of the ELF and PE/COFF versions of the binary > is > + // identical. That way, r_offset will retain its validity as a > PE/COFF > + // image offset, and we can record it in the COFF fixup table > + // unmodified. > + // > + if (shdr->sh_addr != mCoffSectionsOffset[Index]) { > + Error (NULL, 0, 3000, > + "Invalid", "%s: PT_DYNAMIC relocations require identical ELF > and PE/COFF section offsets.", > + mInImageName); > + } > + } > + > for (K = 0; K < RelSize; K += RelElementSize) { > > if (DynamicSegment->p_paddr == 0) { @@ -973,14 +996,7 @@ > WriteRelocations32 ( > break; > > case R_ARM_RABS32: > - TargetSegment = GetPhdrByIndex (ELF32_R_SYM (Rel->r_info) - 1); > - > - // Note: r_offset in a memory address. Convert it to a pointer > in the coff file. > - Targ = mCoffFile + mCoffSectionsOffset[ ELF32_R_SYM( Rel->r_info > ) ] + Rel->r_offset - TargetSegment->p_vaddr; > - > - *(UINT32 *)Targ = *(UINT32 *)Targ + mCoffSectionsOffset > [ELF32_R_SYM( Rel->r_info )]; > - > - CoffAddFixup (mCoffSectionsOffset[ELF32_R_SYM (Rel->r_info)] + > (Rel->r_offset - TargetSegment->p_vaddr), EFI_IMAGE_REL_BASED_HIGHLOW); > + CoffAddFixup (Rel->r_offset, > + EFI_IMAGE_REL_BASED_HIGHLOW); > break; > > default: > -- > 1.9.1 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
