On Thu, Sep 12, 2019 at 06:46:04PM -0700, Kees Cook wrote:
> This combination appears to be bugged since the original introduction
> of hardened usercopy in v4.8. Is this an untested combination until
> now? (I don't usually do tests with CONFIG_DEBUG_VIRTUAL, but I guess
> I will from now on!)
Tricky one because it is only going to trip when someone actually does
this with a highmem page, so if you have a small machine (eg <512MB)
running a 32-bit kernel, you won't hit it.
> Is kmap somewhere "unexpected" in this case? Ah-ha, yes, it seems it is.
> There is even a helper to do the "right" thing as virt_to_page(). This
> seems to be used very rarely in the kernel... is there a page type for
> kmap pages? This seems like a hack, but it fixes it:
I think this is actually the right thing to do. It'd be better if we had
a kmap_to_head_page(), but we don't.
> @@ -227,7 +228,7 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
> if (!virt_addr_valid(ptr))
> return;
>
> - page = virt_to_head_page(ptr);
> + page = compound_head(kmap_to_page((void *)ptr));
>
> if (PageSlab(page)) {
> /* Check slab allocator for flags and size. */
>
>
> What's the right way to "ignore" the kmap range? (i.e. it's not Slab, so
> ignore it here: I can't find a page type nor a "is this kmap?" helper...)
I don't think we want it to be _ignored_ ... if an attempted copy crosses
outside this page boundary, we want it stopped. So I think this patch
is as good as it can be.
