Re: [PATCH] linux-user/mmap.c: fix integer underflow in target_mremap

[Philippe Mathieu-Daudé]

Hi Jonathan.

On 5/19/20 10:11 AM, Stefano Garzarella wrote:
> Hi Jonathan,
> thanks for the patch!
>
> CCing Riku and Laurent.
>
> On Mon, May 18, 2020 at 12:13:41PM -0600, Jonathan Marler wrote:
> > Been a few more days.  Not sure how often I should be pinging.  If this is
> > too much to ping every few days let me know.

Pinging every week is fine. The problem here, as noticed by Stefano, is 
you forgot to Cc the maintainers, so they surely missed your patch.

Last time Riku sent an email to qemu-devel was more than 2 years ago, 
letling Laurent second him, then went MIA:
https://www.mail-archive.com/address@hidden/msg507843.html

I'd say count 1 week starting today.

Regards,

Phil.

> Is not too much, but next time is better to CC the maintainers.
> You can use 'scripts/get_maintainer.pl' to get the list of maintainers
> and reviewers.
>
> Please take a look at https://wiki.qemu.org/Contribute/SubmitAPatch
>
> > On Fri, May 15, 2020 at 7:36 AM Jonathan Marler <address@hidden>
> > wrote:
> >
> > > Been a couple weeks, checking to see if anyone has looked at this.
> > >
> > > On Sat, May 2, 2020 at 5:43 PM Jonathan Marler <address@hidden>
> > > wrote:
> > >
> > > > FYI, I applied this patch to the qemu build that zig uses to run
> > > > non-native tests (
> > > > https://github.com/ziglang/qemu-static/blob/master/patch/mremap-underflow.diff
> > > > )
> > > >
> > > > After applying it, my new code that calls mremap now passes,
> > > > whereas before the fix I was getting a segfault.
> > > >
> > > > On Sat, May 2, 2020 at 10:12 AM Jonathan Marler <address@hidden>
> > > > wrote:
> > > >
> > > > > Fixes: https://bugs.launchpad.net/bugs/1876373
>
> should be "Buglink: https://bugs.launchpad.net/bugs/1876373";
>
> > > > > This code path in mmap occurs when a page size is decreased with
> > > > > mremap.  When a section of pages is shrunk, qemu calls mmap_reserve on the
> > > > > pages that were released.  However, it has the diff operation reversed,
> > > > > subtracting the larger old_size from the smaller new_size.  Instead, it
> > > > > should be subtracting the smaller new_size from the larger old_size.  You
> > > > > can also see in the previous line of the change that this mmap_reserve call
> > > > > only occurs when old_size > new_size.
>
> Please break the lines of the commit message (max 76 charactes per line):
> https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message
>
> Thanks,
> Stefano
>
> > > > > Signed-off-by: Jonathan Marler <address@hidden>
> > > > > ---
> > > > >   linux-user/mmap.c | 2 +-
> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> > > > > index e378033797..caab62909e 100644
> > > > > --- a/linux-user/mmap.c
> > > > > +++ b/linux-user/mmap.c
> > > > > @@ -708,7 +708,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> > > > > old_size,
> > > > >           if (prot == 0) {
> > > > >               host_addr = mremap(g2h(old_addr), old_size, new_size,
> > > > > flags);
> > > > >               if (host_addr != MAP_FAILED && reserved_va && old_size >
> > > > > new_size) {
> > > > > -                mmap_reserve(old_addr + old_size, new_size - old_size);
> > > > > +                mmap_reserve(old_addr + old_size, old_size - new_size);
> > > > >               }
> > > > >           } else {
> > > > >               errno = ENOMEM;
> > > > > --
> > > > > 2.23.1