On 8 June 2017 at 16:06, Russell King - ARM Linux <linux@xxxxxxxxxxxxxxx> wrote:
> On Wed, Jun 07, 2017 at 06:20:52PM +0000, Ard Biesheuvel wrote:
>> The current safe path iterates over each mapping page by page, and
>> kmap()'s each one individually, which is expensive and unnecessary.
>> Instead, let's use kern_addr_valid() to establish on a per-VMA basis
>> whether we may safely derefence them, and do so via its mapping in
>> the VMALLOC region. This can be done safely due to the fact that we
>> are holding the vmap_area_lock spinlock.
>
> This doesn't sound correct if you look at the definition of
> kern_addr_valid(). For example, x86-32 has:
>
> /*
> * kern_addr_valid() is (1) for FLATMEM and (0) for
> * SPARSEMEM and DISCONTIGMEM
> */
> #ifdef CONFIG_FLATMEM
> #define kern_addr_valid(addr) (1)
> #else
> #define kern_addr_valid(kaddr) (0)
> #endif
>
> The majority of architectures simply do:
>
> #define kern_addr_valid(addr) (1)
>
That is interesting, thanks for pointing it out.
The function read_kcore() [which is where the issue I am trying to fix
originates] currently has this logic:
if (kern_addr_valid(start)) {
unsigned long n;
/*
* Using bounce buffer to bypass the
* hardened user copy kernel text checks.
*/
memcpy(buf, (char *) start, tsz);
n = copy_to_user(buffer, buf, tsz);
/*
* We cannot distinguish between fault on source
* and fault on destination. When this happens
* we clear too and hope it will trigger the
* EFAULT again.
*/
if (n) {
if (clear_user(buffer + tsz - n,
n))
return -EFAULT;
}
} else {
if (clear_user(buffer, tsz))
return -EFAULT;
}
and the implementation I looked at [on arm64] happens to be the only
one that does something non-trivial.
> So, the result is that on the majority of architectures, we're now
> going to simply dereference 'addr' with very little in the way of
> checks.
>
Indeed.
> I think this makes these functions racy - the point at which the
> entry is placed onto the vmalloc list is quite different from the
> point where the page table entries for it are populated (which
> happens with the lock dropped.) So, I think this is asking for
> an oops.
>
Fair enough. I will try to find a different approach then.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
