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.
