| Message ID | 20250318173547.59475-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: generate xen.efi image with no write-execute sections | expand |
On 18/03/2025 5:35 pm, 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! > > 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-- > > Account for the program header offset of the LOAD segment when checking > whether the NOTE segments is contained within. > > Fixes: a353cab905af ('build_id: Provide ld-embedded build-ids') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
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 ||. Jan
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. */
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@suse.com>
diff --git a/xen/arch/x86/boot/mkelf32.c b/xen/arch/x86/boot/mkelf32.c index 5f9e7e440e84..91742162bbb1 100644 --- 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 ) { fprintf(stderr, "Expected .note section within .text section!\n" \ "Offset %"PRId64" not within %d!\n",
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! 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-- Account for the program header offset of the LOAD segment when checking whether the NOTE segments is contained within. Fixes: a353cab905af ('build_id: Provide ld-embedded build-ids') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/boot/mkelf32.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)