Skip to content

Fixes for SW PAN

Will Deacon linux-arm-kernel 1 version New

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest0 of 2 patches 0 repliesNew

Patches in v1 No applies recorded yet

None of the 2 patches the cover letter announces are in this archive.

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [PATCH 0/2] Fixes for SW PAN
Quoted 28 lines · Catalin Marinas
Quoted 27 lines · Will Deacon
Quoted 26 lines · Catalin Marinas
Quoted 25 lines · Will Deacon
Quoted 23 lines · Vinayak Menon
Quoted 9 lines · Will Deacon

After lots of collective head scratching in response to Vinayak's mail here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2017-December/545641.html

It turns out that we have a problem with SW PAN and kernel threads, where the saved ttbr0 value for a kernel thread can be stale and subsequently inherited by other kernel threads over a fork.

Quoted 6 lines

These two patches attempt to fix that. We've not be able to reproduce the exact failure reported above, but I added some assertions to the uaccess routines to check for discrepancies between the active_mm pgd and the saved ttbr0 value (ignoring the zero page) and these no longer fire with these changes, but do fire without them if EFI runtime services are enabled on my Seattle board.

Thanks Will. So these 2 patches fix the case of kthreads having a stale saved ttbr0. The callstack I had shared in the original issue description was not of a kthread (its user task with PF_KTHREAD not set. The tsk->mm was set to NULL by exit_mm I think). So do you think this could be a different problem ?
I had a look at the dumps again and what I see is that, the PA part of the saved ttbr0
(from thread_info) is not the same as the pa(tsk->active_mm->pgd). The PA derived from saved ttbr0 actually points to a page which is "now" owned by slab.

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Having not been able to reproduce the failure you described, I can't give you a good answer to this.

Looking at the code (again), if we context switch in do_exit after exit_mm, then the thread behaves an awful lot like a kernel thread: current->mm is NULL and we're in lazy TLB mode. Furthermore, that context switch will drop the last reference to the old mm and the pgd will finally be freed.

So I think my patches will solve your case too because we'll call enter_lazy_tlb again when getting scheduled back in. If you have any way to test them, that would be great.

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

While these fixes make sense for a stable backport, I could go back to per-cpu saved_ttbr0 as in the first version of this patchset:

http://lkml.kernel.org/r/1471015666-23125-4-git-send-email-catalin.marinas at arm.com

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

(changed in v2 for some marginally shorter asm code)

To be honest, if we're going to consider changes as fundamental as that, I'd much prefer for us to use the active_mm->pgd directly, setting the zero page if it's either NULL or init_mm. This would need some hacking for EFI...

The problem is the __pa(active_mm->pgd) and doing it in assembly may be pretty unreadable.

I don't think it would be *that* bad if you can get hold of memstart_addr.

Quoted 1 line

Yet another option is to move save_ttbr0 in mm_context_t.

Perhaps, but I don't dislike the current code as much as I did now that, we understand it better ;)

Will

Read the cover letter in its thread