Re: [PATCH for-8.0 v2 10/12] linux-user: Pass last not end to probe_guest_base

[Philippe Mathieu-Daudé]

On 28/3/23 19:47, Richard Henderson wrote:
> On 3/28/23 06:51, Philippe Mathieu-Daudé wrote:
> > Hi Richard,
> >
> > On 27/3/23 23:18, Richard Henderson wrote:
> > > Pass the address of the last byte of the image, rather than
> > > the first address past the last byte.  This avoids overflow
> > > when the last page of the address space is involved.
> > >
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > ---
> > >   linux-user/elfload.c  | 24 ++++++++++++------------
> > >   linux-user/flatload.c |  2 +-
> > >   2 files changed, 13 insertions(+), 13 deletions(-)


> > > index e99570ca18..5efec2630e 100644
> > > --- a/linux-user/flatload.c
> > > +++ b/linux-user/flatload.c
> > > @@ -448,7 +448,7 @@ static int load_flat_file(struct linux_binprm * 
> > > bprm,
> > >        * Allocate the address space.
> > >        */
> > >       probe_guest_base(bprm->filename, 0,
> > > -                     text_len + data_len + extra + indx_len);
> > > +                     text_len + data_len + extra + indx_len - 1);
> >
> > Per linux-user/user-internals.h:
> >
> > /**
> >   * probe_guest_base:
> >   * @image_name: the executable being loaded
> >   * @loaddr: the lowest fixed address in the executable
> >   * @hiaddr: the highest fixed address in the executable
> >   *
> >   * Creates the initial guest address space in the host memory space.
> >   *
> >   * If @loaddr == 0, then no address in the executable is fixed,
> >   * i.e. it is fully relocatable.  In that case @hiaddr is the size
> >   * of the executable.
> >   *
> >   * This function will not return if a valid value for guest_base
> >   * cannot be chosen.  On return, the executable loader can expect
> >   *
> >   *    target_mmap(loaddr, hiaddr - loaddr, ...)
> >   *
> >   * to succeed.
> >   */
> >
> > Since here @loaddr == 0, "@hiaddr is the size of the executable",
> > not "the first address past the last byte".
> >
> > So we can not "Pass the address of the last byte of the image"
> > to this API. Maybe the API description is incorrect, in that
> > case your patch is right. Otherwise we might need to tune
> > probe_guest_base().
>
> I'll update the documentation.

Then:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>