Re: [PATCH v6 8/8] linux-user: Load pie executables at upper memory

[Helge Deller]

On 8/2/23 20:36, Richard Henderson wrote:
> On 8/1/23 16:27, Helge Deller wrote:
> > +/* where to map binaries? */
> > +#if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> > +# define TASK_UNMAPPED_BASE_PIE 0x5500000000
> > +# define TASK_UNMAPPED_BASE    0x7000000000
> > +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> > +# define TASK_UNMAPPED_BASE_PIE    0x00300000
> > +# define TASK_UNMAPPED_BASE    (GUEST_ADDR_MAX - 0x20000000 + 1)
> > +#else /* HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32 */
> > +# define TASK_UNMAPPED_BASE_PIE    0x00000000
> > +# define TASK_UNMAPPED_BASE    0x40000000
> > +#endif
>
> It would probably be easier to follow if you use the kernel's name:
> ELF_ET_DYN_BASE.

Agreed.

> Should be in linux-user/$GUEST/target_mmap.h with
> TASK_UNMAPPED_BASE, per my comment vs patch 7.

Maybe, but see my comments/answers there...

> > +        /* do sanity checks on guest memory layout */
> > +        if (mmap_next_start >= GUEST_ADDR_MAX) {
> > +            mmap_next_start = GUEST_ADDR_MAX - 0x1000000000 + 1;
> > +        }
>
> What in the world is that random number?

It's random. Idea is to keep enough space for shared libs below GUEST_ADDR_MAX.

> It certainly won't compile
> on 32-bit host, and certainly isn't relevant to a 32-bit guest.

That code will never be compiled/executed on 32-bit guests & hosts.
The defines for TASK_UNMAPPED_BASE take already care of that.
But I agree it's not directly visible (and probably not nice that way).

> > +        if (TASK_UNMAPPED_BASE_PIE >= mmap_next_start) {
> > +            fprintf(stderr, "Memory too small for PIE executables.\n");
> > +            exit(EXIT_FAILURE);
> > +        }
>
> Really bad timing for this diagnostic.  What if a PIE executable isn't even 
> being used?
>
> Both TASK_UNMAPPED_BASE and ELF_ET_DYN_BASE could be computed in main (or a 
> subroutine), when reserved_va is assigned.

Helge