Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 15 May 2023 23:10:18 +0100
From:   Lorenzo Stoakes <lstoakes@gmail.com>
To:     Peter Xu <peterx@redhat.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Mike Rapoport <rppt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        "Liam R . Howlett" <Liam.Howlett@oracle.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>
Subject: Re: [PATCH v2] mm: userfaultfd: avoid passing an invalid range to
 vma_merge()
Message-ID: <a0ad836a-c837-47bb-9c04-cd89a11d6ffa@lucifer.local>
References: <20230515193232.67552-1-lstoakes@gmail.com>
 <ZGKmjUL5etmvIouh@x1n>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZGKmjUL5etmvIouh@x1n>
Precedence: bulk

On Mon, May 15, 2023 at 05:39:25PM -0400, Peter Xu wrote:
> On Mon, May 15, 2023 at 08:32:32PM +0100, Lorenzo Stoakes wrote:
> > As well as fixing the repro described in [1] this also continues to pass
> > uffd unit tests.
>
> Side note on testing, not directly relevant to the patch itself..
>
> I'm wondering whether do we have tests somewhere to just test vma
> operations on split and merge, then verify it using smap or whatever.
>
> The uffd unit test in this case is probably not gonna trigger anything
> because we always mostly register with a whole vma over the testing ranges,
> so not immediately helpful.
>
> The trick here is we have quite a few ops that will call vma merge/split in
> different ways, but logically we can still category them into: (1) add/del
> vmas, or (2) modify vma flags, so at least for case (2) we can have a
> framework to cover all the cases (mbind, mprotect, uffd reg/unreg, mlock,
> etc.), the difference will be the flags we'll be looking at for different
> cases, however how vmas merge/split should be somehow in the same pattern.
>

I totally agree we need more testing on this. We do have some basic
self-tests for various things but I don't think we test this specifically
or certainly not in the way I'd prefer (somehow pull vma_merge() + friends
into userland and instrument with heavy unit tests).

I do intend to try to do something with this soon.

Something with /proc/$pid/[s]maps could be a good straightforward thing, I
will try to write some small test (we already have a little repro for the
reported issue) for this anyway.

> --
> Peter Xu
>