[v5,0/2] Fix issues with vmalloc flush flag

Message

Edgecombe, Rick P May 27, 2019, 9:10 p.m. UTC

These two patches address issues with the recently added
VM_FLUSH_RESET_PERMS vmalloc flag.

Patch 1 addresses an issue that could cause a crash after other
architectures besides x86 rely on this path.

Patch 2 addresses an issue where in a rare case strange arguments
could be provided to flush_tlb_kernel_range(). 

v4->v5:
 - Update commit messages with info that the first issue will actually
   not cause problems today
 - Avoid re-use of variable (PeterZ)

v3->v4:
 - Drop patch that switched vm_unmap_alias() calls to regular flush (Andy)
 - Add patch to address correctness previously fixed in dropped patch

v2->v3:
 - Split into two patches (Andy)

v1->v2:
 - Update commit message with more detail
 - Fix flush end range on !CONFIG_ARCH_HAS_SET_DIRECT_MAP case


Rick Edgecombe (2):
  vmalloc: Fix calculation of direct map addr range
  vmalloc: Avoid rare case of flushing tlb with weird arguments

 mm/vmalloc.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Peter Zijlstra May 28, 2019, 8:01 a.m. UTC | #1

On Mon, May 27, 2019 at 02:10:56PM -0700, Rick Edgecombe wrote:
> These two patches address issues with the recently added
> VM_FLUSH_RESET_PERMS vmalloc flag.
> 
> Patch 1 addresses an issue that could cause a crash after other
> architectures besides x86 rely on this path.
> 
> Patch 2 addresses an issue where in a rare case strange arguments
> could be provided to flush_tlb_kernel_range(). 

Thanks!

David Miller May 29, 2019, 12:23 a.m. UTC | #2

From: Rick Edgecombe <[e-mail]>
Date: Mon, 27 May 2019 14:10:56 -0700

> These two patches address issues with the recently added
> VM_FLUSH_RESET_PERMS vmalloc flag.
> 
> Patch 1 addresses an issue that could cause a crash after other
> architectures besides x86 rely on this path.
> 
> Patch 2 addresses an issue where in a rare case strange arguments
> could be provided to flush_tlb_kernel_range(). 

It just occurred to me another situation that would cause trouble on
sparc64, and that's if someone the address range of the main kernel
image ended up being passed to flush_tlb_kernel_range().

That would flush the locked kernel mapping and crash the kernel
instantly in a completely non-recoverable way.

Edgecombe, Rick P May 29, 2019, 5:11 a.m. UTC | #3

On Tue, 2019-05-28 at 17:23 -0700, David Miller wrote:
> From: Rick Edgecombe <[e-mail]>
> Date: Mon, 27 May 2019 14:10:56 -0700
> 
> > These two patches address issues with the recently added
> > VM_FLUSH_RESET_PERMS vmalloc flag.
> > 
> > Patch 1 addresses an issue that could cause a crash after other
> > architectures besides x86 rely on this path.
> > 
> > Patch 2 addresses an issue where in a rare case strange arguments
> > could be provided to flush_tlb_kernel_range(). 
> 
> It just occurred to me another situation that would cause trouble on
> sparc64, and that's if someone the address range of the main kernel
> image ended up being passed to flush_tlb_kernel_range().
> 
> That would flush the locked kernel mapping and crash the kernel
> instantly in a completely non-recoverable way.

Hmm, I haven't received the logs from Meelis that will show the real
ranges being passed into flush_tlb_kernel_range() on sparc, but it
should be flushing a range spanning from the modules to the direct map.
It looks like the kernel is at the very bottom of the address space, so
not included. Or do you mean the pages that hold the kernel text on the
direct map?

But regardless of this new code, DEBUG_PAGEALLOC hangs with the first
vmalloc free/unmap. That should be just flushing a single allocation in
the vmalloc range.

If it is somehow catching a locked entry though... Are there any sparc
flush mechanisms that could be used in vmalloc that won't touch locked
entries? Peter Z was pointing out that flush_tlb_all() might be more
approriate for vmalloc anyway.