__data_end may not correctly represent text start position when using CFG_WITH_PAGER

[yanyan-wrs]

When configured with pager, non-paged part is appended at end of data section and is moved to correct position during start:

#ifdef CFG_WITH_PAGER
    /*
     * Move init code into correct location and move hashes to a
     * temporary safe location until the heap is initialized.
     *
     * The binary is built as:
     * [Pager code, rodata and data] : In correct location
     * [Init code and rodata] : Should be copied to __text_init_start
     * [Hashes] : Should be saved before initializing pager
     *
     */
    ldr r0, =__text_init_start  /* dst */
    ldr r1, =__data_end     /* src */
    ldr r2, =__tmp_hashes_end   /* dst limit */
    /* Copy backwards (as memmove) in case we're overlapping */
    sub r2, r2, r0      /* len */
    add r0, r0, r2
    add r1, r1, r2
    ldr r2, =__text_init_start
copy_init:
    ldmdb   r1!, {r3, r7-r13}
    stmdb   r0!, {r3, r7-r13}
    cmp r0, r2
    bgt copy_init
#endif

However, sometimes the __data_end is not the position. The image is built as:
[pager code, rodate and data] + [Init code + others]
the linker script puts all data sections together and after that is __data_end. However, if the length of the last few data sections are 0x0, and if some of the data sections are 4 bytes aligned, it moves the __data_end to 4 bytes boundary, but the actual length of the data section (i.e., the position where to concatenate the text section) does not move forward.

An example is shown below:

 .data.freelist
                0x4e0111c4       0x10 out/arm-plat-imx/core-lib/libutils/libutils.a(bget_malloc.o)
 .data.__malloc_mu
                0x4e0111d4       0x18 out/arm-plat-imx/core-lib/libutils/libutils.a(malloc_lock.o)
                0x4e0111d4                __malloc_mu
                0x4e0111ec                . = ALIGN (0x4)

.igot.plt       0x4e0111ec        0x0
 .igot.plt      0x00000000        0x0 out/arm-plat-imx/core/arch/arm/kernel/tee_ta_manager.o

.ctors          0x4e0111f0        0x0
                0x4e0111f0                __ctor_list = .
 *(SORT(.ctors) SORT(.ctors.*) SORT(.init_array) SORT(.init_array.*))
                0x4e0111f0                __ctor_end = .

.dtors          0x4e0111f0        0x0
                0x4e0111f0                __dtor_list = .
 *(SORT(.dtors) SORT(.dtors.*) SORT(.fini_array) SORT(.fini_array.*))
                0x4e0111f0                __dtor_end = .

.got
 *(SORT(.got.plt))
 *(SORT(.got))

.dynamic
 *(SORT(.dynamic))
                0x4e0111f0                __data_end = .

Here the end of the data is at 0x4e0111ec, while the next .ctors advance the end to 0x4e0111f0 due to alignment requirement. The resultant __data_end is also 0x4e0111f0. Unfortunately, when building the tee.bin, the objdump does not care about the empty data sections so the actual binary end at 0x4e0111ec, from which the text are appended, see hexdump of tee.bin:

00111f0 0000 0000 0000 0000 0000 0000 ffff ffff
0011200 0000 0000 0000 0000 b480 b083 af00 6078
0011210 687b 68db 4618 370c 46bd f85d 7b04 4770

the text b048 b083 should begin at 0x1120c (0x111f0+0x1C OPTEE header), but it begins at 0x11208.

[jenswi-linaro]

Yes, this is a bug.

[jenswi-linaro]

In #1044 there's a patch (core: arm: kern.ld.S: consistent 8 bytes alignment) that may fix this.

[yanyan-wrs]

I think we can solve this problem by padding the tee-pager.bin to the size of __data_end:

This is the original linker script to produce the tee-pager.bin:

pageable_sections := .*_pageable
init_sections := .*_init
cleanfiles += $(link-out-dir)/tee-pager.bin
$(link-out-dir)/tee-pager.bin: $(link-out-dir)/tee.elf 
    @$(cmd-echo-silent) '  OBJCOPY $@'
    $(q)$(OBJCOPYcore) -O binary \
        --remove-section="$(pageable_sections)" \
        --remove-section="$(init_sections)" \
        $< $@

This is the fixed one, adding the --pad-to line in objcopy:

pageable_sections := .*_pageable
init_sections := .*_init
cleanfiles += $(link-out-dir)/tee-pager.bin
$(link-out-dir)/tee-pager.bin: $(link-out-dir)/tee.elf \
                        $(link-out-dir)/tee-data_end.txt
    @$(cmd-echo-silent) '  OBJCOPY $@'
    $(q)$(OBJCOPYcore) -O binary \
        --remove-section="$(pageable_sections)" \
        --remove-section="$(init_sections)" \
        --pad-to `cat $(link-out-dir)/tee-data_end.txt` \
        $< $@

and __data_end is obtained by NM the ELF, as usual

cleanfiles += $(link-out-dir)/tee-data_end.txt
$(link-out-dir)/tee-data_end.txt: $(link-out-dir)/tee.elf
    @$(cmd-echo-silent) '  GEN     $@'
    @echo -n 0x > $@
    $(q)$(NMcore) $< | grep __data_end | sed 's/ .*$$//' >> $@

[yanyan-wrs]

And the real cause is this, .ctors/.dtor sections are 8 bytes aligned, and they are empty sections; so if previous section does not end at 8 bytes boundary, it moves the current address forward, while it does not fill any content because it is empty. The OBJDUMP utility only dumps content to the end of previous non-empty sections. Adding --pad-to can solve this problem:

 .ctors : ALIGN(8) {
  __ctor_list = .;
  KEEP(*(.ctors .ctors.* .init_array .init_array.*))
  __ctor_end = .;
 }
 .dtors : ALIGN(8) {
  __dtor_list = .;
  KEEP(*(.dtors .dtors.* .fini_array .fini_array.*))
  __dtor_end = .;
 }

[jenswi-linaro]

I like this approach. Can you make a pull request based on this?

[jforissier]

Closing this issue since the proposed change has been merged. Feel free to re-open it if needed.