Re: [PATCH v2 7/9] bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag

From: Warner Losh <imp@bsdimp.com>
To: Guy Yur <guyyur@gmail.com>
Cc: "Kyle Evans" <kevans@freebsd.org>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [PATCH v2 7/9] bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag
Date: Sun, 26 Sep 2021 18:17:50 -0600	[thread overview]
Message-ID: <CANCZdfp_9V9+yNLFiNdPF=Na+pwfhFcqbBk1jAJHz5Woa0fNHw@mail.gmail.com> (raw)
In-Reply-To: <b5d1b686-a6ce-63b0-8655-4918f70efdf6@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]

On Sun, Sep 26, 2021 at 1:07 PM Guy Yur <guyyur@gmail.com> wrote:

> On 26/9/21 20:08, Warner Losh wrote:
> >
> >
> > On Fri, Sep 24, 2021 at 6:00 AM Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >
> >     On 9/21/21 9:56 PM, Warner Losh wrote:
> >     >           /* no page was there, so we allocate one */
> >     >           void *p = mmap(host_start, qemu_host_page_size, prot,
> >     > -                       flags | MAP_ANON, -1, 0);
> >     > +                       flags | ((fd != -1) ? MAP_ANON : 0), -1,
> 0);
> >
> >     I don't understand this change, given that the actual fd passed is
> >     always -1.
> >
> >
> > That's a very good question. I'll have to trace down why that was made
> > because
> > I'm having trouble with it as well now that I'm trying to defend it.
> >
> mmap_frag can be called with a valid fd, if flags doesn't contain one of
> MAP_ANON, MAP_STACK, MAP_GUARD.
> The passed fd to mmap is -1 but if flags contains MAP_GUARD then
> MAP_ANON cannot be added.
>
> * If fd is valid (not -1) we want to map the pages with MAP_ANON.
> * If flags contains MAP_GUARD we don't want to add MAP_ANON because it
> will be rejected.
> https://github.com/freebsd/freebsd-src/blob/master/sys/vm/vm_mmap.c#L302
> * If flags contains MAP_ANON it doesn't matter if we add it or not.
> * If flags contains MAP_STACK, mmap adds MAP_ANON when called so doesn't
> matter if we add it or not either.
> https://github.com/freebsd/freebsd-src/blob/master/sys/vm/vm_mmap.c#L284
>
> The intention was to not pass MAP_ANON for the flags that use fd == -1
> without specifying the flags directly,
> with the assumption that future flags that don't require fd will also
> not require MAP_ANON.
> Changing to !(flags & MAP_GUARD) will also work.
>

Thanks Guy. that fills in the missing pieces for me of the range of
possibilities
for calling. I've added it as a comment since it's tricky enough I've had
to ask
twice and Richard asked as well :). It will be in the next spin of the mmap
series.

> Guy Yur
>
> > Warner
> >
> >
> >     r~
> >
>

[-- Attachment #2: Type: text/html, Size: 3220 bytes --]