arm64: mark kernel text segment as MEMBLOCK_NOMAP

Commit 752af28bd711 ("arm64: move kernel image to base of vmalloc area") moves the mapping of the kernel text and data segments out of the linear region, and takes care not to create a writable alias of the read-only kernel text segment by checking each memblock against overlap when the memblocks are mapped into the linear mapping.

However, it is more correct, and much simpler, to mark the [_stext, _etext] interval as MEMBLOCK_NOMAP. This will also prevent the interval from being omitted from the linear region, but this fact will now also be reflected in the output of pfn_valid(), and so code that expects any pfn_valid() page to be mapped and accessible (which is a reasonable assumption) does not get surprised by the text segment being inaccessible via the linear mapping.

Signed-off-by Ard Biesheuvel <[email protected]>

---

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

Quoted 14 lines · Ard Biesheuvel

Commit 752af28bd711 ("arm64: move kernel image to base of vmalloc area") moves the mapping of the kernel text and data segments out of the linear region, and takes care not to create a writable alias of the read-only kernel text segment by checking each memblock against overlap when the memblocks are mapped into the linear mapping.

However, it is more correct, and much simpler, to mark the [_stext, _etext] interval as MEMBLOCK_NOMAP. This will also prevent the interval from being omitted from the linear region, but this fact will now also be reflected in the output of pfn_valid(), and so code that expects any pfn_valid() page to be mapped and accessible (which is a reasonable assumption) does not get surprised by the text segment being inaccessible via the linear mapping.

Quoted 6 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

I have another branch with your kaslr patches but I didn't dare to merge them into for-next/core until we sort out the first sub-series.

Quoted 27 lines · Catalin Marinas

Quoted 14 lines · Ard Biesheuvel

Commit 752af28bd711 ("arm64: move kernel image to base of vmalloc area") moves the mapping of the kernel text and data segments out of the linear region, and takes care not to create a writable alias of the read-only kernel text segment by checking each memblock against overlap when the memblocks are mapped into the linear mapping.

However, it is more correct, and much simpler, to mark the [_stext, _etext] interval as MEMBLOCK_NOMAP. This will also prevent the interval from being omitted from the linear region, but this fact will now also be reflected in the output of pfn_valid(), and so code that expects any pfn_valid() page to be mapped and accessible (which is a reasonable assumption) does not get surprised by the text segment being inaccessible via the linear mapping.

Quoted 6 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

Quoted 5 lines

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

Agreed.

Quoted 3 lines

I have another branch with your kaslr patches but I didn't dare to merge them into for-next/core until we sort out the first sub-series.

Yes, I noticed. As I pointed out in its cover letter, I expected the first subseries to be the most problematic in terms of fallout in other areas, and I turned out to be right, unfortunately. The coverage so far has been quite useful tbh, especially since I apparently never tested initrd.

If you are going to drop it for now anyway, I can do a v6sub1 with this issue and the initrd issue addressed, but also an issue with the KVM ksym ref patch with GCC 4.8.

Since these issues affect
bisectability, I'd prefer to rebase entirely, and fold all the fixes rather than apply them on top.

Quoted 9 lines · Catalin Marinas

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

If you are going to drop it for now anyway, I can do a v6sub1 with this issue and the initrd issue addressed, but also an issue with the KVM ksym ref patch with GCC 4.8. Since these issues affect bisectability, I'd prefer to rebase entirely, and fold all the fixes rather than apply them on top.

Quoted 10 lines · Ard Biesheuvel

Quoted 9 lines · Catalin Marinas

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

[...]

Quoted 5 lines

If you are going to drop it for now anyway, I can do a v6sub1 with this issue and the initrd issue addressed, but also an issue with the KVM ksym ref patch with GCC 4.8. Since these issues affect bisectability, I'd prefer to rebase entirely, and fold all the fixes rather than apply them on top.

That's fine. I'll push one more patch to the for-next/pgtable branch, converting __set_fixmap_offset() back to a macro as it broke other architectures. Otherwise it is not rebased and I'll merge it into for-next core.

Quoted 18 lines · Catalin Marinas

Quoted 10 lines · Ard Biesheuvel

Quoted 9 lines · Catalin Marinas

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

[...]

Quoted 5 lines

If you are going to drop it for now anyway, I can do a v6sub1 with this issue and the initrd issue addressed, but also an issue with the KVM ksym ref patch with GCC 4.8. Since these issues affect bisectability, I'd prefer to rebase entirely, and fold all the fixes rather than apply them on top.

Quoted 4 lines

That's fine. I'll push one more patch to the for-next/pgtable branch, converting __set_fixmap_offset() back to a macro as it broke other architectures. Otherwise it is not rebased and I'll merge it into for-next core.

I have identified (and fixed) a Kasan issue in my patches: instead of leaving a hole for the 64 MB module window in the Kasan shadow region, I populated it with zero mappings, which caused many strange errors, and since the vmap() routines don't expect block mappings (which are created for the kernel image shadow by vmemmap_populate()),

I guess false positive kasan warnings (like the ones you were seeing) are also a possible symptom of this issue.

Quoted 10 lines · Ard Biesheuvel

Quoted 9 lines · Catalin Marinas

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

[...]

Quoted 5 lines

If you are going to drop it for now anyway, I can do a v6sub1 with this issue and the initrd issue addressed, but also an issue with the KVM ksym ref patch with GCC 4.8. Since these issues affect bisectability, I'd prefer to rebase entirely, and fold all the fixes rather than apply them on top.

That's fine. I'll push one more patch to the for-next/pgtable branch, converting __set_fixmap_offset() back to a macro as it broke other architectures. Otherwise it is not rebased and I'll merge it into for-next core.

Quoted 18 lines · Catalin Marinas

Quoted 10 lines · Ard Biesheuvel

Quoted 9 lines · Catalin Marinas

Ard, I'm about to drop the kernel memory map patches from -next. There are several issues like KASAN (which may as well be a KASAN bug but I haven't got to the bottom of it yet), initrd (you have patches but require additional acks).

I'll keep your stuff on the for-next/kernmap branch and do further debugging. If it stabilises in the next 1-2 weeks, I'll merge it into for-next/core for 4.6, otherwise, it will have to wait. I would really like these patches merged but it looks like they need wider testing.

[...]

Quoted 5 lines

If you are going to drop it for now anyway, I can do a v6sub1 with this issue and the initrd issue addressed, but also an issue with the KVM ksym ref patch with GCC 4.8. Since these issues affect bisectability, I'd prefer to rebase entirely, and fold all the fixes rather than apply them on top.

Quoted 4 lines

That's fine. I'll push one more patch to the for-next/pgtable branch, converting __set_fixmap_offset() back to a macro as it broke other architectures. Otherwise it is not rebased and I'll merge it into for-next core.

If you are doing for-next/core from scratch, perhaps it is better to squash the fixmap patches as well?

Commit 752af28bd711 ("arm64: move kernel image to base of vmalloc area") moves the mapping of the kernel text and data segments out of the linear region, and takes care not to create a writable alias of the read-only kernel text segment by checking each memblock against overlap when the memblocks are mapped into the linear mapping.

However, it is more correct, and much simpler, to mark the [_stext, _etext] interval as MEMBLOCK_NOMAP. This will also prevent the interval from being omitted from the linear region, but this fact will now also be reflected in the output of pfn_valid(), and so code that expects any pfn_valid() page to be mapped and accessible (which is a reasonable assumption) does not get surprised by the text segment being inaccessible via the linear mapping.

Signed-off-by Ard Biesheuvel <[email protected]>

---

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

Hi Ard,

Quoted 14 lines · Ard Biesheuvel

Commit 752af28bd711 ("arm64: move kernel image to base of vmalloc area") moves the mapping of the kernel text and data segments out of the linear region, and takes care not to create a writable alias of the read-only kernel text segment by checking each memblock against overlap when the memblocks are mapped into the linear mapping.

However, it is more correct, and much simpler, to mark the [_stext, _etext] interval as MEMBLOCK_NOMAP. This will also prevent the interval from being omitted from the linear region, but this fact will now also be reflected in the output of pfn_valid(), and so code that expects any pfn_valid() page to be mapped and accessible (which is a reasonable assumption) does not get surprised by the text segment being inaccessible via the linear mapping.

Quoted 2 lines

Signed-off-by Ard Biesheuvel <[email protected]>

---

With this patch on arm64/for-next/kernmap, I get the following KVM-related boot failure on Juno:
-----------------------------%<-----------------------------
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
hw perfevents: enabled with armv8_pmuv3 PMU driver, 7 counters available

------------[ cut here ]------------
kernel BUG at ../arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:577!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:
CPU: 4 PID: 1 Comm: swapper/0 Not tainted 4.5.0-rc1+ #2022
Hardware name: ARM Juno development board (r1) (DT)
task: ffffffc976cf8000 ti: ffffffc976d60000 task.ti: ffffffc976d60000
PC is at create_hyp_mappings+0x144/0x148
LR is at create_hyp_mappings+0x7c/0x148

[SNIP]

[<ffffff80040abc14>] create_hyp_mappings+0x144/0x148
[<ffffff80040a9be8>] kvm_arch_init+0x198/0x53c
[<ffffff800409fe24>] kvm_init+0x38/0x290
[<ffffff80040a84e4>] arm_init+0x24/0x2c
[<ffffff8004082994>] do_one_initcall+0x94/0x198
[<ffffff8004a22b10>] kernel_init_freeable+0x148/0x1e8
[<ffffff800473e344>] kernel_init+0x20/0xe4
[<ffffff8004085cd0>] ret_from_fork+0x10/0x40

-----------------------------%<-----------------------------

The offending function is:
-----------------------------%<----------------------------- static phys_addr_t kvm_kaddr_to_phys(void *kaddr)

{
        if (!is_vmalloc_addr(kaddr)) {
                BUG_ON(!virt_addr_valid(kaddr));
                return __pa(kaddr);
        } else {
                return page_to_phys(vmalloc_to_page(kaddr)) +
                       offset_in_page(kaddr);
        }
}

-----------------------------%<-----------------------------

When the BUG_ON() fires, kaddr points into the hole in the linear map left by the kernel text.

Quoted 4 lines

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

It does, now I have to work out how best to fix hibernate!

Quoted 5 lines · James Morse

Quoted 3 lines · Ard Biesheuvel

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

Quoted 2 lines

It does, now I have to work out how best to fix hibernate!

Indeed. The alternative would be to map the [_stext, _etext] interval into the linear region as a ro/nx alias of the actual mapping in the vmalloc region. That would probably make your life easier, but I am not convinced the overall result is better.

Quoted 6 lines · Ard Biesheuvel

Quoted 5 lines · James Morse

Quoted 3 lines · Ard Biesheuvel

This should hopefully address the issue reported by James, but I suppose more work is required on the hibernate side to ensure the unmapped text region is preserved, since it is no longer covered by the linear mapping.

Quoted 7 lines

Quoted 2 lines

It does, now I have to work out how best to fix hibernate!

Indeed. The alternative would be to map the [_stext, _etext] interval into the linear region as a ro/nx alias of the actual mapping in the vmalloc region. That would probably make your life easier, but I am not convinced the overall result is better.

I assumed you would NACK any approach that looked like that:
It could be done temporarily while the memory snapshot is being taken, only swsusp_save() would see this mapping, it can be removed again afterwards. But pfn_valid() will still say those pages aren't there, and there is no memblock_clear_nomap() to change this.

The current approach involves copying the kernel text into pages that are saved, and having two lists of pages that need restoring. (wheels within wheels!) Pretty unpleasant, and requires another patch in the core code, so likely won't be possible for v4.6. I should have the patches out for review later today.