[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/7] x86/mkelf32: account for offset when detecting note segment placement
On 19.03.2025 15:16, Roger Pau Monné wrote: > On Wed, Mar 19, 2025 at 11:07:33AM +0100, Jan Beulich wrote: >> On 18.03.2025 18:35, Roger Pau Monne wrote: >>> mkelf32 attempt to check that the program header defined NOTE segment falls >>> inside of the LOAD segment, as the build-id should be loaded for Xen at >>> runtime to check. >>> >>> However the current code doesn't take into account the LOAD program header >>> segment offset when calculating overlap with the NOTE segment. This >>> results in incorrect detection, and the following build error: >>> >>> arch/x86/boot/mkelf32 --notes xen-syms ./.xen.elf32 0x200000 \ >>> `nm xen-syms | sed -ne 's/^\([^ ]*\) . >>> __2M_rwdata_end$/0x\1/p'` >>> Expected .note section within .text section! >>> Offset 4244776 not within 2910364! >> >> Not your fault, but: Such printing of decimal numbers is of course very >> unhelpful when ... >> >>> When xen-syms has the following program headers: >>> >>> Program Header: >>> LOAD off 0x0000000000200000 vaddr 0xffff82d040200000 paddr >>> 0x0000000000200000 align 2**21 >>> filesz 0x00000000002c689c memsz 0x00000000003f7e20 flags rwx >>> NOTE off 0x000000000040c528 vaddr 0xffff82d04040c528 paddr >>> 0x000000000040c528 align 2**2 >>> filesz 0x0000000000000024 memsz 0x0000000000000024 flags r-- >> >> ... any half-way sane tool prints such values in hex. >> >>> --- a/xen/arch/x86/boot/mkelf32.c >>> +++ b/xen/arch/x86/boot/mkelf32.c >>> @@ -358,7 +358,8 @@ int main(int argc, char **argv) >>> note_sz = in64_phdr.p_memsz; >>> note_base = in64_phdr.p_vaddr - note_base; >>> >>> - if ( in64_phdr.p_offset > dat_siz || offset > in64_phdr.p_offset ) >>> + if ( in64_phdr.p_offset > (offset + dat_siz) || >>> + offset > in64_phdr.p_offset ) >> >> This is an improvement, so fine to go in with Andrew's ack, but it still >> doesn't match what the error message suggests is wanted: This checks only >> whether .note starts after .text or ends before .text. A partial overlap, >> which isn't okay either aiui, would go through fine. >> >> Oh, and - even in your change there's an off-by-1: You mean >= in the lhs >> of the ||. > > Hm, I see, it would be better as: > > diff --git a/xen/arch/x86/boot/mkelf32.c b/xen/arch/x86/boot/mkelf32.c > index 5f9e7e440e84..f0f406687cea 100644 > --- a/xen/arch/x86/boot/mkelf32.c > +++ b/xen/arch/x86/boot/mkelf32.c > @@ -358,11 +358,13 @@ int main(int argc, char **argv) > note_sz = in64_phdr.p_memsz; > note_base = in64_phdr.p_vaddr - note_base; > > - if ( in64_phdr.p_offset > dat_siz || offset > in64_phdr.p_offset ) > + if ( in64_phdr.p_offset < offset || > + in64_phdr.p_offset + in64_phdr.p_filesz > offset + dat_siz ) > { > fprintf(stderr, "Expected .note section within .text section!\n" > \ > - "Offset %"PRId64" not within %d!\n", > - in64_phdr.p_offset, dat_siz); > + ".note: [%"PRIx64", %"PRIx64") .text: [%x, %x)\n", > + in64_phdr.p_offset, in64_phdr.p_offset + > in64_phdr.p_filesz, > + offset, offset + dat_siz); > return 1; > } > /* Gets us the absolute offset within the .text section. */ Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |