[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 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. */
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |