Corentin, On Sat, Dec 05 2020 at 19:43, Corentin Labbe wrote: > On Fri, Dec 04, 2020 at 09:58:21PM +0100, Thomas Gleixner wrote: >> Can you please replace the debug patch with the one below and try again? >> That stops the trace right on the condition. > > Hello, the result could be found at http://kernel.montjoie.ovh/130739.log Thanks for providing this. This is clearly showing where stuff goes wrong. It starts here at 729.550001. I removed the uninteresting parts: 0d..2 147103293us : __kmap_local_page_prot <-sg_miter_next 0d..3 147103308us :__kmap_local_pfn_prot: kmap_local_pfn: 1 ffefd000 0d..3 147103311us : __kmap_local_page_prot <-sg_miter_next 0d..4 147103325us : __kmap_local_pfn_prot: kmap_local_pfn: 3 ffefb000 0d..3 147103429us : kunmap_local_indexed <-sg_miter_stop 0d..4 147103433us : kunmap_local_indexed: kunmap_local: 3 ffefd000 So this maps two pages and unmaps the first one. That's all called from sun4i_ss_opti_poll() and the bug is clearly visible there: sg_miter_next(&mi); sg_miter_next(&mo); release_ss: sg_miter_stop(&mi); sg_miter_stop(&mo); Written by yourself :) Same issue in sun4i_ss_cipher_poll() Fix below. Julia, it might be worth to have a coccinelle check for that. It's the only place which got it wrong, but this goes unnoticed when code is e.g. only fully tested on 64bit or as in this case never tested with full debugging enabled. The whole kmap_atomic and kmap_local (new in next) family and all users like the sg_miter stuff are affected by this. Thanks, tglx --- Subject: crypto: sun4i-ss - Fix sg_miter_stop() ordering From: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Date: Sat, 05 Dec 2020 20:17:28 +0100 sun4i_ss_opti_poll() and sun4i_ss_cipher_poll() do: sg_miter_next(&mi); sg_miter_next(&mo); ... sg_miter_stop(&mi); sg_miter_stop(&mo); which is the wrong order because sg_miter_next() maps a page with kmap_atomic() and sg_miter_stop() unmaps it. kmap_atomic() uses a stack internaly which requires that the nested map is unmapped first. As this uses the wrong order it triggers the warning in kunmap_local_indexed() which checks the to be unmapped address and subsequently crashes. This went unnoticed for 5 years because the ARM kmap_atomic() implementation had the warning conditional on CONFIG_DEBUG_HIGHMEM which was obviously never enabled when testing that driver. Flip the order to cure it. Reported-by: Corentin Labbe <clabbe.montjoie@xxxxxxxxx> Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator") Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Cc: stable@xxxxxxxxxxxxxxx --- drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c @@ -109,8 +109,8 @@ static int noinline_for_stack sun4i_ss_o } release_ss: - sg_miter_stop(&mi); sg_miter_stop(&mo); + sg_miter_stop(&mi); writel(0, ss->base + SS_CTL); spin_unlock_irqrestore(&ss->slock, flags); return err; @@ -333,8 +333,8 @@ static int sun4i_ss_cipher_poll(struct s } release_ss: - sg_miter_stop(&mi); sg_miter_stop(&mo); + sg_miter_stop(&mi); writel(0, ss->base + SS_CTL); spin_unlock_irqrestore(&ss->slock, flags);
