| Message ID | 20221009083050.3814850-1-panqinglin2020@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9f2ac64d6ca60db99132e08628ac2899f956a0ec |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | [v1,1/1] riscv: mm: add missing memcpy in kasan_init | expand |
On Sun, Oct 09, 2022 at 04:30:50PM +0800, panqinglin2020@iscas.ac.cn wrote: > From: Qinglin Pan <panqinglin2020@iscas.ac.cn> > > Hi Atish, > > It seems that the panic is due to the missing memcpy during kasan_init. > Could you please check whether this patch is helpful? If this does solve the problem it would be: Fixes: 8fbdccd2b173 ("riscv: mm: Support kasan for sv57") right? Thanks, Conor. > > When doing kasan_populate, the new allocated base_pud/base_p4d should > contain kasan_early_shadow_{pud, p4d}'s content. Add the missing memcpy > to avoid page fault when read/write kasan shadow region. > > Tested on: > - qemu with sv57 and CONFIG_KASAN on. > - qemu with sv48 and CONFIG_KASAN on. > > Signed-off-by: Qinglin Pan <panqinglin2020@iscas.ac.cn> > --- > arch/riscv/mm/kasan_init.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c > index a22e418dbd82..e1226709490f 100644 > --- a/arch/riscv/mm/kasan_init.c > +++ b/arch/riscv/mm/kasan_init.c > @@ -113,6 +113,8 @@ static void __init kasan_populate_pud(pgd_t *pgd, > base_pud = pt_ops.get_pud_virt(pfn_to_phys(_pgd_pfn(*pgd))); > } else if (pgd_none(*pgd)) { > base_pud = memblock_alloc(PTRS_PER_PUD * sizeof(pud_t), PAGE_SIZE); > + memcpy(base_pud, (void *)kasan_early_shadow_pud, > + sizeof(pud_t) * PTRS_PER_PUD); > } else { > base_pud = (pud_t *)pgd_page_vaddr(*pgd); > if (base_pud == lm_alias(kasan_early_shadow_pud)) { > @@ -173,8 +175,11 @@ static void __init kasan_populate_p4d(pgd_t *pgd, > base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(*pgd))); > } else { > base_p4d = (p4d_t *)pgd_page_vaddr(*pgd); > - if (base_p4d == lm_alias(kasan_early_shadow_p4d)) > + if (base_p4d == lm_alias(kasan_early_shadow_p4d)) { > base_p4d = memblock_alloc(PTRS_PER_PUD * sizeof(p4d_t), PAGE_SIZE); > + memcpy(base_p4d, (void *)kasan_early_shadow_p4d, > + sizeof(p4d_t) * PTRS_PER_P4D); > + } > } > > p4dp = base_p4d + p4d_index(vaddr); > -- > 2.35.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 10/9/22 7:30 PM, Conor Dooley wrote: > On Sun, Oct 09, 2022 at 04:30:50PM +0800, panqinglin2020@iscas.ac.cn wrote: >> From: Qinglin Pan <panqinglin2020@iscas.ac.cn> >> >> Hi Atish, >> >> It seems that the panic is due to the missing memcpy during kasan_init. >> Could you please check whether this patch is helpful? > > If this does solve the problem it would be: > Fixes: 8fbdccd2b173 ("riscv: mm: Support kasan for sv57") > right? > > Thanks, > Conor. > Hi Conor, Thanks a lot for notification! I have change the title and resend it. Thanks, Qinglin.
On Sun, Oct 09, 2022 at 09:25:31PM +0800, Qinglin Pan wrote: > On 10/9/22 7:30 PM, Conor Dooley wrote: > > On Sun, Oct 09, 2022 at 04:30:50PM +0800, panqinglin2020@iscas.ac.cn wrote: > > > From: Qinglin Pan <panqinglin2020@iscas.ac.cn> > > > > > > Hi Atish, > > > > > > It seems that the panic is due to the missing memcpy during kasan_init. > > > Could you please check whether this patch is helpful? > > > > If this does solve the problem it would be: > > Fixes: 8fbdccd2b173 ("riscv: mm: Support kasan for sv57") > > right? > > > > Thanks, > > Conor. > > > > Hi Conor, > > Thanks a lot for notification! > I have change the title and resend it. Unfortunately I was not suggesting a new title for the patch.. A "Fixes:" tag goes as part of the sign off block to show what commit a given patch fixes. Your original title looks fine to me. Thanks, Conor.
On Sun, Oct 9, 2022 at 1:31 AM <panqinglin2020@iscas.ac.cn> wrote: > > From: Qinglin Pan <panqinglin2020@iscas.ac.cn> > > Hi Atish, > > It seems that the panic is due to the missing memcpy during kasan_init. > Could you please check whether this patch is helpful? > > When doing kasan_populate, the new allocated base_pud/base_p4d should > contain kasan_early_shadow_{pud, p4d}'s content. Add the missing memcpy > to avoid page fault when read/write kasan shadow region. > > Tested on: > - qemu with sv57 and CONFIG_KASAN on. > - qemu with sv48 and CONFIG_KASAN on. > > Signed-off-by: Qinglin Pan <panqinglin2020@iscas.ac.cn> > --- > arch/riscv/mm/kasan_init.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c > index a22e418dbd82..e1226709490f 100644 > --- a/arch/riscv/mm/kasan_init.c > +++ b/arch/riscv/mm/kasan_init.c > @@ -113,6 +113,8 @@ static void __init kasan_populate_pud(pgd_t *pgd, > base_pud = pt_ops.get_pud_virt(pfn_to_phys(_pgd_pfn(*pgd))); > } else if (pgd_none(*pgd)) { > base_pud = memblock_alloc(PTRS_PER_PUD * sizeof(pud_t), PAGE_SIZE); > + memcpy(base_pud, (void *)kasan_early_shadow_pud, > + sizeof(pud_t) * PTRS_PER_PUD); > } else { > base_pud = (pud_t *)pgd_page_vaddr(*pgd); > if (base_pud == lm_alias(kasan_early_shadow_pud)) { > @@ -173,8 +175,11 @@ static void __init kasan_populate_p4d(pgd_t *pgd, > base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(*pgd))); > } else { > base_p4d = (p4d_t *)pgd_page_vaddr(*pgd); > - if (base_p4d == lm_alias(kasan_early_shadow_p4d)) > + if (base_p4d == lm_alias(kasan_early_shadow_p4d)) { > base_p4d = memblock_alloc(PTRS_PER_PUD * sizeof(p4d_t), PAGE_SIZE); > + memcpy(base_p4d, (void *)kasan_early_shadow_p4d, > + sizeof(p4d_t) * PTRS_PER_P4D); > + } > } > > p4dp = base_p4d + p4d_index(vaddr); > -- > 2.35.1 > Yes. This patch fixes the boot issue for me with Kasan enabled on v6.0. Tested-by: Atish Patra <atishp@rivosinc.com> Thanks for the patch. Few nit comments: You can drop the message addressed to me in the commit text. Usually, that should be after the last sign off between two "---" As conor suggested, there should be a Fixes tag[1] in the commit text. [1] https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#:~:text=A%20Fixes%3A%20tag%20indicates%20that,versions%20should%20receive%20your%20fix.
On Sun, 09 Oct 2022 23:49:36 PDT (-0700), atishp@atishpatra.org wrote: > On Sun, Oct 9, 2022 at 1:31 AM <panqinglin2020@iscas.ac.cn> wrote: >> >> From: Qinglin Pan <panqinglin2020@iscas.ac.cn> >> >> Hi Atish, >> >> It seems that the panic is due to the missing memcpy during kasan_init. >> Could you please check whether this patch is helpful? >> >> When doing kasan_populate, the new allocated base_pud/base_p4d should >> contain kasan_early_shadow_{pud, p4d}'s content. Add the missing memcpy >> to avoid page fault when read/write kasan shadow region. >> >> Tested on: >> - qemu with sv57 and CONFIG_KASAN on. >> - qemu with sv48 and CONFIG_KASAN on. >> >> Signed-off-by: Qinglin Pan <panqinglin2020@iscas.ac.cn> >> --- >> arch/riscv/mm/kasan_init.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c >> index a22e418dbd82..e1226709490f 100644 >> --- a/arch/riscv/mm/kasan_init.c >> +++ b/arch/riscv/mm/kasan_init.c >> @@ -113,6 +113,8 @@ static void __init kasan_populate_pud(pgd_t *pgd, >> base_pud = pt_ops.get_pud_virt(pfn_to_phys(_pgd_pfn(*pgd))); >> } else if (pgd_none(*pgd)) { >> base_pud = memblock_alloc(PTRS_PER_PUD * sizeof(pud_t), PAGE_SIZE); >> + memcpy(base_pud, (void *)kasan_early_shadow_pud, >> + sizeof(pud_t) * PTRS_PER_PUD); >> } else { >> base_pud = (pud_t *)pgd_page_vaddr(*pgd); >> if (base_pud == lm_alias(kasan_early_shadow_pud)) { >> @@ -173,8 +175,11 @@ static void __init kasan_populate_p4d(pgd_t *pgd, >> base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(*pgd))); >> } else { >> base_p4d = (p4d_t *)pgd_page_vaddr(*pgd); >> - if (base_p4d == lm_alias(kasan_early_shadow_p4d)) >> + if (base_p4d == lm_alias(kasan_early_shadow_p4d)) { >> base_p4d = memblock_alloc(PTRS_PER_PUD * sizeof(p4d_t), PAGE_SIZE); >> + memcpy(base_p4d, (void *)kasan_early_shadow_p4d, >> + sizeof(p4d_t) * PTRS_PER_P4D); >> + } >> } >> >> p4dp = base_p4d + p4d_index(vaddr); >> -- >> 2.35.1 >> > > Yes. This patch fixes the boot issue for me with Kasan enabled on v6.0. > > Tested-by: Atish Patra <atishp@rivosinc.com> > > Thanks for the patch. Few nit comments: > > You can drop the message addressed to me in the commit text. > Usually, that should be after the last sign off between two "---" > > As conor suggested, there should be a Fixes tag[1] in the commit text. > > [1] https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html#:~:text=A%20Fixes%3A%20tag%20indicates%20that,versions%20should%20receive%20your%20fix. I've got this on not-for-next in a cleaned up state, so no reason to resend just for these. It also fixes my boot issues, but it's not quite passing the smell test right now. I've looked at this a few times and every time I do I manage to convince myself that there's a bunch of issues in these kasan initialization routines, but my attempts to clean them up just result in more breakages. So I think there's something I don't quite get about this yet. I'll try and find some time this weekend to dig into this further, hopefully it's just all the merge window interrupts that are preventing me from getting anywhere.
On Sun, 9 Oct 2022 16:30:50 +0800, panqinglin2020@iscas.ac.cn wrote: > From: Qinglin Pan <panqinglin2020@iscas.ac.cn> > > Hi Atish, > > It seems that the panic is due to the missing memcpy during kasan_init. > Could you please check whether this patch is helpful? > > [...] Applied, thanks! [1/1] riscv: mm: add missing memcpy in kasan_init https://git.kernel.org/palmer/c/9f2ac64d6ca6 Best regards,
diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c index a22e418dbd82..e1226709490f 100644 --- a/arch/riscv/mm/kasan_init.c +++ b/arch/riscv/mm/kasan_init.c @@ -113,6 +113,8 @@ static void __init kasan_populate_pud(pgd_t *pgd, base_pud = pt_ops.get_pud_virt(pfn_to_phys(_pgd_pfn(*pgd))); } else if (pgd_none(*pgd)) { base_pud = memblock_alloc(PTRS_PER_PUD * sizeof(pud_t), PAGE_SIZE); + memcpy(base_pud, (void *)kasan_early_shadow_pud, + sizeof(pud_t) * PTRS_PER_PUD); } else { base_pud = (pud_t *)pgd_page_vaddr(*pgd); if (base_pud == lm_alias(kasan_early_shadow_pud)) { @@ -173,8 +175,11 @@ static void __init kasan_populate_p4d(pgd_t *pgd, base_p4d = pt_ops.get_p4d_virt(pfn_to_phys(_pgd_pfn(*pgd))); } else { base_p4d = (p4d_t *)pgd_page_vaddr(*pgd); - if (base_p4d == lm_alias(kasan_early_shadow_p4d)) + if (base_p4d == lm_alias(kasan_early_shadow_p4d)) { base_p4d = memblock_alloc(PTRS_PER_PUD * sizeof(p4d_t), PAGE_SIZE); + memcpy(base_p4d, (void *)kasan_early_shadow_p4d, + sizeof(p4d_t) * PTRS_PER_P4D); + } } p4dp = base_p4d + p4d_index(vaddr);