Re: [PATCH v2 3/3] fork: lock VMAs of the parent process when forking

[Suren Baghdasaryan <surenb@google.com>]

On Fri, Aug 4, 2023 at 6:17 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> On 8/5/23, Suren Baghdasaryan <surenb@google.com> wrote:
> > On Fri, Aug 4, 2023 at 5:49 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
> >> However, the other users (that I know of ) go through the mmap
> >> semaphore to mess with anything which means they will wait for
> >> dup_mmap to finish (or do their work first). I would be surprised if
> >> there were any cases which don't take the semaphore, given that it was
> >> a requirement prior to the vma patchset (unless you patched some to no
> >> longer need it?). I would guess worst case the semaphore can be added
> >> if missing.
> >
> > No, the only mmap_lock read-lock that is affected is during the page
> > fault, which is expected.
> >
>
> I have difficulty parsing your statement.

I was just saying that vma lock patchset did not touch any other
mmap_locking paths except for the page fault one where we try to skip
read-locking mmap_lock.

>
> I am saying that any 3rd parties which can trigger page faults already
> read lock mmap_lock or can be made to do it (and I don't know any case
> which does not already, but I'm not willing to spend time poking
> around to make sure). One can consider 3rd parties as not a problem,
> modulo the audit.
>
> Past that there does is no known source of trouble? In my original
> e-mail I was worried about processes up the chain in ancestry, perhaps
> some of the state is shared(?) and the locking at hand neuters any
> problems. I'm guessing this is not necessary.
>
> Bottom line though it looks like this will work fine?
>
> That said, I'm not going to submit a patch I can't confidently defend.
> As I did not dig into any of the VMA code and can't be arsed to audit
> all places which mess with "foreign" mm, I'm definitely not submitting
> this myself. You are most welcome to write your own variant at your
> leisure. :)

Ok, I see. I'll need to double check locking when a 3rd party is
involved. Will post a patch when I'm confident enough it's safe.
Thanks!

>
> --
> Mateusz Guzik <mjguzik gmail.com>