Re: [PATCH] usercopy: Skip HIGHMEM page checking

[Matthew Wilcox <willy@infradead.org>]

On Mon, Sep 16, 2019 at 08:05:00PM -0700, Kees Cook wrote:
> On Mon, Sep 16, 2019 at 05:32:09PM -0700, Matthew Wilcox wrote:
> > On Mon, Sep 16, 2019 at 02:32:56PM -0700, Kees Cook wrote:
> > > When running on a system with >512MB RAM with a 32-bit kernel built with:
> > > 
> > > 	CONFIG_DEBUG_VIRTUAL=y
> > > 	CONFIG_HIGHMEM=y
> > > 	CONFIG_HARDENED_USERCOPY=y
> > > 
> > > all execve()s will fail due to argv copying into kmap()ed pages, and on
> > > usercopy checking the calls ultimately of virt_to_page() will be looking
> > > for "bad" kmap (highmem) pointers due to CONFIG_DEBUG_VIRTUAL=y:
> > 
> > I don't understand why you want to skip the check.  We must not cross a
> > page boundary of a kmapped page.
> 
> That requires a new test which hasn't existed before. First I need to
> fix the bug, and then we can add a new test and get that into -next,
> etc.

I suppose that depends where your baseline is.  From the perspective
of "before Kees added this feature", your point of view makes sense.
From the perspective of "what's been shipping for the last six months",
this is a case which has simply not happened before now (or we'd've seen
a bug report).

I don't think you need to change anything for check_page_span() to do
the right thing.  The rodata/data/bss checks will all fall through.
If the copy has the correct bounds, the 'wholly within one base page'
check will pass and it'll return.  If the copy does span a page,
the virt_to_head_page(end) call will return something bogus, then the
PageReserved and CMA test will cause the usercopy_abort() test to fail.

So I think your first patch is the right patch.