Will do, thanks!
On Thu, 26 Jul 2018 at 19:12, Laurent Vivier <laur...@vivier.eu> wrote:
> Le 26/07/2018 à 19:58, Alex Bennée a écrit :
> >
> > Laurent Vivier <laur...@vivier.eu> writes:
> >
> >> Le 26/07/2018 à 15:29, Alex Bennée a écrit:
> >>> I've slightly re-organised the check to more closely match the
> >>> sequence that the kernel uses in do_mmap().
> >>>
> >>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> >>> Cc: umarcor <1783...@bugs.launchpad.net>
> >>> ---
> >>> linux-user/mmap.c | 14 +++++++++++---
> >>> 1 file changed, 11 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> >>> index d0c50e4888..3ef69fa2d0 100644
> >>> --- a/linux-user/mmap.c
> >>> +++ b/linux-user/mmap.c
> >>> @@ -391,14 +391,22 @@ abi_long target_mmap(abi_ulong start, abi_ulong
> len, int prot,
> >>> }
> >>> #endif
> >>>
> >>> - if (offset & ~TARGET_PAGE_MASK) {
> >>> + if (!len) {
> >>> errno = EINVAL;
> >>> goto fail;
> >>> }
> >>>
> >>> len = TARGET_PAGE_ALIGN(len);
> >>> - if (len == 0)
> >>> - goto the_end;
> >>> + if (!len) {
> >>> + errno = EINVAL;
> >>> + goto fail;
> >>> + }
> >>
> >> Why do you check twice len?
> >> TARGET_PAGE_ALIGN() rounds up the value, so if it was not 0, it cannot
> >> be now.
> >
> > I was trying to follow the kernel style but I realise TARGET_PAGE_ALIGN
> > might be a different test compared to the kernel's PAGE_ALIGN(len) for
> > overflows:
> ...
> > /* Careful about overflows.. */
> > len = PAGE_ALIGN(len);
> > if (!len)
> > return -ENOMEM;
> >
>
>
> OK, so keep it but you should use ENOMEM, not EINVAL (and add a comment :)
> )
>
> Thanks,
> Laurent
>
--
Alex Bennée
KVM/QEMU Hacker for Linaro
