flush_cache_vmap() must be called after new vmalloc mappings are
installed in the page table in order to allow architectures to make sure
the new mapping is visible.
Fixes: 3e9a9e256b1e ("mm: add a vmap_pfn function")
Reported-by: Dylan Jhong <[email protected]>
Closes: https://lore.kernel.org/linux-riscv/[email protected]/
Signed-off-by: Alexandre Ghiti <[email protected]>
---
mm/vmalloc.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 93cf99aba335..228a4a5312f2 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2979,6 +2979,10 @@ void *vmap_pfn(unsigned long *pfns, unsigned int count, pgprot_t prot)
free_vm_area(area);
return NULL;
}
+
+ flush_cache_vmap((unsigned long)area->addr,
+ (unsigned long)area->addr + count * PAGE_SIZE);
+
return area->addr;
}
EXPORT_SYMBOL_GPL(vmap_pfn);
--
2.39.2
On Wed, 9 Aug 2023 18:46:33 +0200 Alexandre Ghiti <[email protected]> wrote:
> flush_cache_vmap() must be called after new vmalloc mappings are
> installed in the page table in order to allow architectures to make sure
> the new mapping is visible.
Thanks. What are the user-visible effects of this bug?
On Wed, Aug 09, 2023 at 06:46:33PM +0200, Alexandre Ghiti wrote:
> flush_cache_vmap() must be called after new vmalloc mappings are
> installed in the page table in order to allow architectures to make sure
> the new mapping is visible.
Looks good. I somehow vaguely remember seing a patch like this floating
around before as part of a series, but if that didn't make it it
certainly should now.
Reviewed-by: Christoph Hellwig <[email protected]>
Hi Andrew,
On Wed, Aug 9, 2023 at 8:46 PM Andrew Morton <[email protected]> wrote:
>
> On Wed, 9 Aug 2023 18:46:33 +0200 Alexandre Ghiti <[email protected]> wrote:
>
> > flush_cache_vmap() must be called after new vmalloc mappings are
> > installed in the page table in order to allow architectures to make sure
> > the new mapping is visible.
>
> Thanks. What are the user-visible effects of this bug?
It could lead to a panic since on some architectures (like powerpc),
the page table walker could see the wrong pte value and trigger a
spurious page fault that can not be resolved (see commit f1cb8f9beba8
("powerpc/64s/radix: avoid ptesync after set_pte and
ptep_set_access_flags")).
But actually the patch is aiming at riscv: the riscv specification
allows the caching of invalid entries in the TLB, and since we
recently removed the vmalloc page fault handling, we now need to emit
a tlb shootdown whenever a new vmalloc mapping is emitted
(https://lore.kernel.org/linux-riscv/[email protected]/).
That's a temporary solution, there are ways to avoid that :)
Thanks,
Alex
On Wed, Aug 09, 2023 at 06:46:33PM +0200, Alexandre Ghiti wrote:
> flush_cache_vmap() must be called after new vmalloc mappings are
> installed in the page table in order to allow architectures to make sure
> the new mapping is visible.
>
> Fixes: 3e9a9e256b1e ("mm: add a vmap_pfn function")
> Reported-by: Dylan Jhong <[email protected]>
> Closes: https://lore.kernel.org/linux-riscv/[email protected]/
> Signed-off-by: Alexandre Ghiti <[email protected]>
> ---
> mm/vmalloc.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 93cf99aba335..228a4a5312f2 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2979,6 +2979,10 @@ void *vmap_pfn(unsigned long *pfns, unsigned int count, pgprot_t prot)
> free_vm_area(area);
> return NULL;
> }
> +
> + flush_cache_vmap((unsigned long)area->addr,
> + (unsigned long)area->addr + count * PAGE_SIZE);
> +
> return area->addr;
> }
> EXPORT_SYMBOL_GPL(vmap_pfn);
> --
> 2.39.2
>
Hi Alex,
Looks good to me. Thanks.
Reviewed-by: Dylan Jhong <[email protected]>
Best regards,
Dylan Jhong
On Wed, 09 Aug 2023 15:25:19 PDT (-0700), Christoph Hellwig wrote:
> On Wed, Aug 09, 2023 at 06:46:33PM +0200, Alexandre Ghiti wrote:
>> flush_cache_vmap() must be called after new vmalloc mappings are
>> installed in the page table in order to allow architectures to make sure
>> the new mapping is visible.
>
> Looks good. I somehow vaguely remember seing a patch like this floating
> around before as part of a series, but if that didn't make it it
> certainly should now.
>
> Reviewed-by: Christoph Hellwig <[email protected]>
I think we're likely to end up with performance problems around here,
but at least it's correct. If someone has performance
Dylan: this fixes your breakage as well, right?
I've queued it up for testing, but I doubt QEMU would find any issues
here. My build box has been slow lately, but it should end up in fixes
later today.
On Thu, 10 Aug 2023 08:13:56 PDT (-0700), Palmer Dabbelt wrote:
> On Wed, 09 Aug 2023 15:25:19 PDT (-0700), Christoph Hellwig wrote:
>> On Wed, Aug 09, 2023 at 06:46:33PM +0200, Alexandre Ghiti wrote:
>>> flush_cache_vmap() must be called after new vmalloc mappings are
>>> installed in the page table in order to allow architectures to make sure
>>> the new mapping is visible.
>>
>> Looks good. I somehow vaguely remember seing a patch like this floating
>> around before as part of a series, but if that didn't make it it
>> certainly should now.
>>
>> Reviewed-by: Christoph Hellwig <[email protected]>
>
> I think we're likely to end up with performance problems around here,
> but at least it's correct. If someone has performance
>
> Dylan: this fixes your breakage as well, right?
>
> I've queued it up for testing, but I doubt QEMU would find any issues
> here. My build box has been slow lately, but it should end up in fixes
> later today.
Sorry about that, I'm in the wrong thread -- I meant to be over here
<https://lore.kernel.org/all/[email protected]/>.
Reviewed-by: Palmer Dabbelt <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
but I'm not taking this via the RISC-V tree unless someone asks.