On 08/13/2018 04:50 PM, Logan Gunthorpe wrote:
On 13/08/18 05:48 PM, Kit Chow wrote:
On 08/13/2018 04:39 PM, Logan Gunthorpe wrote:
On 13/08/18 05:30 PM, Kit Chow wrote:
In arch/x86/include/asm/page.h, there is the following comment in
regards to validating the virtual address.
/*
* virt_to_page(kaddr) returns a valid pointer if and only if
* virt_addr_valid(kaddr) returns true.
*/
#define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
So it looks like the validation by virt_addr_valid was somehow dropped
from the virt_to_page code path. Does anyone have any ideas what
happended to it?
I don't think it was ever validated (though I haven't been around long
enough to say for certain). What the comment is saying is that you
shouldn't rely on virt_to_page() unless you know virt_addr_valid() is
true (which in most cases you can without the extra check). virt_to_page
is meant to be really fast so adding an extra validation would probably
be a significant performance regression for the entire kernel.
The fact that this can happen through dma_map_single() is non-ideal at
best. It assumes the caller is mapping regular memory and doesn't check
this at all. It may make sense to fix that but I think people expect
dma_map_single() to be as fast as possible as well...
Perhaps include the validation with some debug turned on?
The problem is how often do you develop code with any of the debug
config options turned on?
There's already a couple of BUG_ONs in dma_map_single so maybe another
one with virt_addr_valid wouldn't be so bad.
Had my very first Linux crash on the dma direction BUG_ON when I tried
DMA_NONE :).