| Message ID | 696386a-84f8-b33c-82e5-f865ed6eb39@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: allow pte_offset_map[_lock]() to fail | expand |
On Thu, Jun 08, 2023 at 06:21:41PM -0700, Hugh Dickins wrote: > vmalloc_to_page() was using pte_offset_map() (followed by pte_unmap()), > but it's intended for userspace page tables: prefer pte_offset_kernel(). > > Signed-off-by: Hugh Dickins <hughd@google.com> > Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com> Currently Linus' tree is reliably failing to boot on pine64plus, an arm64 SBC. Most other boards seem fine, though I am seeing some additional instability on Tritium which is another Allwinner platform, I've not dug into that yet and Tritium is generally less stable. We end up seeing NULL or otherwise bad pointer dereferences, the specific error does vary a bit though it mostly appears to be in the pinctrl code. A bisect (full log below) identified this patch as introducing the failure, nothing is jumping out at me about the patch and it's not affecting everything so I'd not be surprised if it's just unconvering some bug in the platform support but I'm not super familiar with the code. Sample backtrace: [ 1.919725] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 [ 1.928551] Mem abort info: [ 1.931359] ESR = 0x0000000096000044 ... [ 1.968870] [0000000000000000] user address but active_mm is swapper ... [ 2.093969] Call trace: [ 2.096414] dt_remember_or_free_map+0xc8/0x120 [ 2.100949] pinctrl_dt_to_map+0x23c/0x364 [ 2.105050] create_pinctrl+0x68/0x3ec [ 2.108803] pinctrl_get+0xb0/0x124 [ 2.112294] devm_pinctrl_get+0x48/0x90 [ 2.116133] pinctrl_bind_pins+0x58/0x158 [ 2.120148] really_probe+0x54/0x2b0 [ 2.123724] __driver_probe_device+0x78/0x12c Another common theme is the same but with an address like 0x4c and: [ 2.098328] __kmem_cache_alloc_node+0x1bc/0x2dc [ 2.102947] kmalloc_trace+0x20/0x2c [ 2.106524] pinctrl_register_mappings+0x98/0x178 Full boot log from a failure: https://lava.sirena.org.uk/scheduler/job/712456 git bisect start # bad: [06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5] Linux 6.5-rc1 git bisect bad 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 # good: [6995e2de6891c724bfeb2db33d7b87775f913ad1] Linux 6.4 git bisect good 6995e2de6891c724bfeb2db33d7b87775f913ad1 # bad: [1b722407a13b7f8658d2e26917791f32805980a2] Merge tag 'drm-next-2023-06-29' of git://anongit.freedesktop.org/drm/drm git bisect bad 1b722407a13b7f8658d2e26917791f32805980a2 # bad: [3a8a670eeeaa40d87bd38a587438952741980c18] Merge tag 'net-next-6.5' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next git bisect bad 3a8a670eeeaa40d87bd38a587438952741980c18 # bad: [6e17c6de3ddf3073741d9c91a796ee696914d8a0] Merge tag 'mm-stable-2023-06-24-19-15' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm git bisect bad 6e17c6de3ddf3073741d9c91a796ee696914d8a0 # good: [2605e80d3438c77190f55b821c6575048c68268e] Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux git bisect good 2605e80d3438c77190f55b821c6575048c68268e # good: [72dc6db7e3b692f46f3386b8dd5101d3f431adef] Merge tag 'wq-for-6.5-cleanup-ordered' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq git bisect good 72dc6db7e3b692f46f3386b8dd5101d3f431adef # bad: [179d3e4f3bfa5947821c1b1bc6aa49a4797b7f21] mm/madvise: clean up force_shm_swapin_readahead() git bisect bad 179d3e4f3bfa5947821c1b1bc6aa49a4797b7f21 # good: [523716770e63e229dbb6307d663f03d990dfefc5] maple_tree: rework mtree_alloc_{range,rrange}() git bisect good 523716770e63e229dbb6307d663f03d990dfefc5 # good: [b764253c18821da31c49a260f92f5d093cf1637e] selftests/mm: fix "warning: expression which evaluates to zero..." in mlock2-tests.c git bisect good b764253c18821da31c49a260f92f5d093cf1637e # good: [5c7f3bf04a6cf266567fdea1ae4987875e92619f] s390: allow pte_offset_map_lock() to fail git bisect good 5c7f3bf04a6cf266567fdea1ae4987875e92619f # good: [0d940a9b270b9220dcff74d8e9123c9788365751] mm/pgtable: allow pte_offset_map[_lock]() to fail git bisect good 0d940a9b270b9220dcff74d8e9123c9788365751 # bad: [0d1c81edc61e553ed7a5db18fb8074c8b78e1538] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel() git bisect bad 0d1c81edc61e553ed7a5db18fb8074c8b78e1538 # good: [2798bbe75b9c2752b46d292e5c2a49f49da36418] mm/page_vma_mapped: pte_offset_map_nolock() not pte_lockptr() git bisect good 2798bbe75b9c2752b46d292e5c2a49f49da36418 # good: [be872f83bf571f4f9a0ac25e2c9c36e905a36619] mm/pagewalk: walk_pte_range() allow for pte_offset_map() git bisect good be872f83bf571f4f9a0ac25e2c9c36e905a36619 # good: [e5ad581c7f1c32d309ae4e895eea0cd1a3d9f363] mm/vmwgfx: simplify pmd & pud mapping dirty helpers git bisect good e5ad581c7f1c32d309ae4e895eea0cd1a3d9f363 # first bad commit: [0d1c81edc61e553ed7a5db18fb8074c8b78e1538] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel()
On Mon, Jul 10, 2023 at 03:42:31PM +0100, Mark Brown wrote: > On Thu, Jun 08, 2023 at 06:21:41PM -0700, Hugh Dickins wrote: > > vmalloc_to_page() was using pte_offset_map() (followed by pte_unmap()), > > but it's intended for userspace page tables: prefer pte_offset_kernel(). > > > > Signed-off-by: Hugh Dickins <hughd@google.com> > > Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com> > > Currently Linus' tree is reliably failing to boot on pine64plus, an > arm64 SBC. Most other boards seem fine, though I am seeing some > additional instability on Tritium which is another Allwinner platform, > I've not dug into that yet and Tritium is generally less stable. > > We end up seeing NULL or otherwise bad pointer dereferences, the > specific error does vary a bit though it mostly appears to be in the > pinctrl code. A bisect (full log below) identified this patch as > introducing the failure, nothing is jumping out at me about the patch > and it's not affecting everything so I'd not be surprised if it's just > unconvering some bug in the platform support but I'm not super familiar > with the code. Yeah seems likely. Do you have a .config you can share for this board? For a 64-bit device you'd expect that this change would probably be a nop. > > Sample backtrace: > > [ 1.919725] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 > [ 1.928551] Mem abort info: > [ 1.931359] ESR = 0x0000000096000044 > > ... > > [ 1.968870] [0000000000000000] user address but active_mm is swapper > > ... > > [ 2.093969] Call trace: > [ 2.096414] dt_remember_or_free_map+0xc8/0x120 > [ 2.100949] pinctrl_dt_to_map+0x23c/0x364 > [ 2.105050] create_pinctrl+0x68/0x3ec > [ 2.108803] pinctrl_get+0xb0/0x124 > [ 2.112294] devm_pinctrl_get+0x48/0x90 > [ 2.116133] pinctrl_bind_pins+0x58/0x158 > [ 2.120148] really_probe+0x54/0x2b0 > [ 2.123724] __driver_probe_device+0x78/0x12c > > Another common theme is the same but with an address like 0x4c and: > > [ 2.098328] __kmem_cache_alloc_node+0x1bc/0x2dc > [ 2.102947] kmalloc_trace+0x20/0x2c > [ 2.106524] pinctrl_register_mappings+0x98/0x178 > > Full boot log from a failure: > > https://lava.sirena.org.uk/scheduler/job/712456 > > git bisect start > # bad: [06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5] Linux 6.5-rc1 > git bisect bad 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 > # good: [6995e2de6891c724bfeb2db33d7b87775f913ad1] Linux 6.4 > git bisect good 6995e2de6891c724bfeb2db33d7b87775f913ad1 > # bad: [1b722407a13b7f8658d2e26917791f32805980a2] Merge tag 'drm-next-2023-06-29' of git://anongit.freedesktop.org/drm/drm > git bisect bad 1b722407a13b7f8658d2e26917791f32805980a2 > # bad: [3a8a670eeeaa40d87bd38a587438952741980c18] Merge tag 'net-next-6.5' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next > git bisect bad 3a8a670eeeaa40d87bd38a587438952741980c18 > # bad: [6e17c6de3ddf3073741d9c91a796ee696914d8a0] Merge tag 'mm-stable-2023-06-24-19-15' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm > git bisect bad 6e17c6de3ddf3073741d9c91a796ee696914d8a0 > # good: [2605e80d3438c77190f55b821c6575048c68268e] Merge tag 'arm64-upstream' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux > git bisect good 2605e80d3438c77190f55b821c6575048c68268e > # good: [72dc6db7e3b692f46f3386b8dd5101d3f431adef] Merge tag 'wq-for-6.5-cleanup-ordered' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq > git bisect good 72dc6db7e3b692f46f3386b8dd5101d3f431adef > # bad: [179d3e4f3bfa5947821c1b1bc6aa49a4797b7f21] mm/madvise: clean up force_shm_swapin_readahead() > git bisect bad 179d3e4f3bfa5947821c1b1bc6aa49a4797b7f21 > # good: [523716770e63e229dbb6307d663f03d990dfefc5] maple_tree: rework mtree_alloc_{range,rrange}() > git bisect good 523716770e63e229dbb6307d663f03d990dfefc5 > # good: [b764253c18821da31c49a260f92f5d093cf1637e] selftests/mm: fix "warning: expression which evaluates to zero..." in mlock2-tests.c > git bisect good b764253c18821da31c49a260f92f5d093cf1637e > # good: [5c7f3bf04a6cf266567fdea1ae4987875e92619f] s390: allow pte_offset_map_lock() to fail > git bisect good 5c7f3bf04a6cf266567fdea1ae4987875e92619f > # good: [0d940a9b270b9220dcff74d8e9123c9788365751] mm/pgtable: allow pte_offset_map[_lock]() to fail > git bisect good 0d940a9b270b9220dcff74d8e9123c9788365751 > # bad: [0d1c81edc61e553ed7a5db18fb8074c8b78e1538] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel() > git bisect bad 0d1c81edc61e553ed7a5db18fb8074c8b78e1538 > # good: [2798bbe75b9c2752b46d292e5c2a49f49da36418] mm/page_vma_mapped: pte_offset_map_nolock() not pte_lockptr() > git bisect good 2798bbe75b9c2752b46d292e5c2a49f49da36418 > # good: [be872f83bf571f4f9a0ac25e2c9c36e905a36619] mm/pagewalk: walk_pte_range() allow for pte_offset_map() > git bisect good be872f83bf571f4f9a0ac25e2c9c36e905a36619 > # good: [e5ad581c7f1c32d309ae4e895eea0cd1a3d9f363] mm/vmwgfx: simplify pmd & pud mapping dirty helpers > git bisect good e5ad581c7f1c32d309ae4e895eea0cd1a3d9f363 > # first bad commit: [0d1c81edc61e553ed7a5db18fb8074c8b78e1538] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel()
On Mon, Jul 10, 2023 at 06:18:27PM +0100, Lorenzo Stoakes wrote: > On Mon, Jul 10, 2023 at 03:42:31PM +0100, Mark Brown wrote: > > We end up seeing NULL or otherwise bad pointer dereferences, the > > specific error does vary a bit though it mostly appears to be in the > > pinctrl code. A bisect (full log below) identified this patch as > > introducing the failure, nothing is jumping out at me about the patch > > and it's not affecting everything so I'd not be surprised if it's just > > unconvering some bug in the platform support but I'm not super familiar > > with the code. > Yeah seems likely. Do you have a .config you can share for this board? For > a 64-bit device you'd expect that this change would probably be a nop. It's definitely happening with arm64 defconfig, possibly with other configs but that's the main one.
On Mon, 10 Jul 2023, Mark Brown wrote: > On Mon, Jul 10, 2023 at 06:18:27PM +0100, Lorenzo Stoakes wrote: > > On Mon, Jul 10, 2023 at 03:42:31PM +0100, Mark Brown wrote: > > > > We end up seeing NULL or otherwise bad pointer dereferences, the > > > specific error does vary a bit though it mostly appears to be in the > > > pinctrl code. A bisect (full log below) identified this patch as > > > introducing the failure, nothing is jumping out at me about the patch > > > and it's not affecting everything so I'd not be surprised if it's just > > > unconvering some bug in the platform support but I'm not super familiar > > > with the code. > > > Yeah seems likely. Do you have a .config you can share for this board? For > > a 64-bit device you'd expect that this change would probably be a nop. > > It's definitely happening with arm64 defconfig, possibly with other > configs but that's the main one. I'm sorry for dropping you in it, Mark, but I'm totally baffled. I've spent most of the day trying to come up with ideas, but failed. I've no doubt that you're seeing what you're seeing, but how it comes about is a mystery. Lorenzo is right that the change should be a no-op - compared with 6.4. But it's not quite a no-op in this series, because 04/32 0d940a9b270b ("mm/pgtable: allow pte_offset_map[_lock]() to fail") diverts the old pte_offset_map() macro off to a new function in mm/pgtable-generic.c; then this commit restores it back to being the pte_offset_kernel() macro. So the asm in vmalloc_to_page() is expected to change in this commit, but change back to what it would have been in 6.4. This feels like one of those bugs which depends on the code size in some way (a bit like those bugs we used to have, where a function was mistakenly marked __init, then in some configs its code landed on a page which got freed at startup - I'm not saying this is that at all, just saying it feels weird in that way). Yet your bisection converges convincingly, which I wouldn't expect in that case. I suppose I should ask you to try reverting this 0d1c81edc61e alone from 6.5-rc1: the consistency of your bisection implies that it will "fix" the issues, and it is a commit which we could drop. It makes me a little nervous, applying userspace-pagetable validation to kernel pagetables, so I don't want to drop it; and it would really be cargo- culting to drop it without understanding. But we could drop it. I guess it would be interesting to know whether vmalloc_to_page() is ever even called in your kernel, before it crashes on the pinctrl stuff. But putting in a printk to report on that may change everything. And I guess it would be interesting to know (from a DEBUG_INFO build of the crashing kernel) which line of dt_remember_or_free_map() it oopses on i.e. which pointer is NULL when it shouldn't be - or maybe you already worked that out. And what device (which ->dt_node_to_map) is involved. If one of the many dt_node_to_map's fails to initialize *map to NULL when it should, and has relied on it happening to be a NULL on the stack already... that might explain it. Another thing to try, would be the kernel at 0d940a9b270b^, just before pte_offset_map() grew a function call: there's a faint possibility that the bug came in before this series, that 0d940a9b270b somehow masked it (I don't see how: vmalloc_to_page() does sensible validation itself), and then 0d1c81edc61e unmasked it again - so that the bisection skipped over, and converged on the wrong point. But I'm thrashing about: I have no confidence that any of this info will help us. Sorry for wasting your time. Thanks, Hugh
[CCing the regression list, as it should be in the loop for regressions: https://docs.kernel.org/admin-guide/reporting-regressions.html] [TLDR: I'm adding this report to the list of tracked Linux kernel regressions; the text you find below is based on a few templates paragraphs you might have encountered already in similar form. See link in footer if these mails annoy you.] On 10.07.23 16:42, Mark Brown wrote: > On Thu, Jun 08, 2023 at 06:21:41PM -0700, Hugh Dickins wrote: >> vmalloc_to_page() was using pte_offset_map() (followed by pte_unmap()), >> but it's intended for userspace page tables: prefer pte_offset_kernel(). >> >> Signed-off-by: Hugh Dickins <hughd@google.com> >> Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com> > > Currently Linus' tree is reliably failing to boot on pine64plus, an > arm64 SBC. Most other boards seem fine, though I am seeing some > additional instability on Tritium which is another Allwinner platform, > I've not dug into that yet and Tritium is generally less stable. > > We end up seeing NULL or otherwise bad pointer dereferences, the > [...] > # first bad commit: [0d1c81edc61e553ed7a5db18fb8074c8b78e1538] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel() Thanks for the report. To be sure the issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression tracking bot: #regzbot ^introduced 0d1c81edc61e553ed7a5db18fb8074c8 #regzbot title mm/vmalloc: NULL or otherwise bad pointer dereferences on ARM64 #regzbot ignore-activity This isn't a regression? This issue or a fix for it are already discussed somewhere else? It was fixed already? You want to clarify when the regression started to happen? Or point out I got the title or something else totally wrong? Then just reply and tell me -- ideally while also telling regzbot about it, as explained by the page listed in the footer of this mail. Developers: When fixing the issue, remember to add 'Link:' tags pointing to the report (the parent of this mail). See page linked in footer for details. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you.
On Mon, Jul 10, 2023 at 09:34:42PM -0700, Hugh Dickins wrote: > This feels like one of those bugs which depends on the code size in > some way (a bit like those bugs we used to have, where a function was > mistakenly marked __init, then in some configs its code landed on a > page which got freed at startup - I'm not saying this is that at all, > just saying it feels weird in that way). > Yet your bisection converges convincingly, which I wouldn't expect > in that case. Yes, it smells like code size or something other than the commit itself, I have seen this sort of behaviour before where something nearby in history introduced something which was then triggered by whatever the bisect points at. > I suppose I should ask you to try reverting this 0d1c81edc61e alone > from 6.5-rc1: the consistency of your bisection implies that it will > "fix" the issues, and it is a commit which we could drop. It makes > me a little nervous, applying userspace-pagetable validation to kernel > pagetables, so I don't want to drop it; and it would really be cargo- > culting to drop it without understanding. But we could drop it. I did look at that, it doesn't revert cleanly by itself. Your other suggestions are all good - I'll poke at them. My suspicion is that there's some longer standing breakage elsewhere and your series (or even just this patch) just happens to push into happening reliably, had it not been a mm change and a memory related bug I'd probably have just discounted the bisect result.
On Tue, 11 Jul 2023, Mark Brown wrote: > On Mon, Jul 10, 2023 at 09:34:42PM -0700, Hugh Dickins wrote: > > > I suppose I should ask you to try reverting this 0d1c81edc61e alone > > from 6.5-rc1: the consistency of your bisection implies that it will > > "fix" the issues, and it is a commit which we could drop. It makes > > me a little nervous, applying userspace-pagetable validation to kernel > > pagetables, so I don't want to drop it; and it would really be cargo- > > culting to drop it without understanding. But we could drop it. > > I did look at that, it doesn't revert cleanly by itself. ... Right, that ptep_get() wrapper on the next line came in on top. The patch to revert just 0d1c81edc61e is this: --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -703,10 +703,11 @@ struct page *vmalloc_to_page(const void if (WARN_ON_ONCE(pmd_bad(*pmd))) return NULL; - ptep = pte_offset_kernel(pmd, addr); + ptep = pte_offset_map(pmd, addr); pte = ptep_get(ptep); if (pte_present(pte)) page = pte_page(pte); + pte_unmap(ptep); return page; }
On Tue, Jul 11, 2023 at 09:13:18AM -0700, Hugh Dickins wrote: > On Tue, 11 Jul 2023, Mark Brown wrote: > > On Mon, Jul 10, 2023 at 09:34:42PM -0700, Hugh Dickins wrote: > > > I suppose I should ask you to try reverting this 0d1c81edc61e alone > > > from 6.5-rc1: the consistency of your bisection implies that it will > > > "fix" the issues, and it is a commit which we could drop. It makes > > > me a little nervous, applying userspace-pagetable validation to kernel > > > pagetables, so I don't want to drop it; and it would really be cargo- > > > culting to drop it without understanding. But we could drop it. > > I did look at that, it doesn't revert cleanly by itself. ... > Right, that ptep_get() wrapper on the next line came in on top. > The patch to revert just 0d1c81edc61e is this: Thanks, tried that and it's still exploding in a similar way (though this time inside a regulator call from the pinctrl code which was happening in other cases).
On Tue, Jul 11, 2023 at 09:13:18AM -0700, Hugh Dickins wrote: > On Tue, 11 Jul 2023, Mark Brown wrote: > > On Mon, Jul 10, 2023 at 09:34:42PM -0700, Hugh Dickins wrote: > > > > > I suppose I should ask you to try reverting this 0d1c81edc61e alone > > > from 6.5-rc1: the consistency of your bisection implies that it will > > > "fix" the issues, and it is a commit which we could drop. It makes > > > me a little nervous, applying userspace-pagetable validation to kernel > > > pagetables, so I don't want to drop it; and it would really be cargo- > > > culting to drop it without understanding. But we could drop it. > > > > I did look at that, it doesn't revert cleanly by itself. ... > > Right, that ptep_get() wrapper on the next line came in on top. > The patch to revert just 0d1c81edc61e is this: Still investigating but I'm pretty convinced this is nothing to do with your commit/series and is just common or garden memory corruption that just happens to get tickled by your changes. Sorry for the noise.
On 11.07.23 19:57, Mark Brown wrote: > On Tue, Jul 11, 2023 at 09:13:18AM -0700, Hugh Dickins wrote: >> On Tue, 11 Jul 2023, Mark Brown wrote: >>> On Mon, Jul 10, 2023 at 09:34:42PM -0700, Hugh Dickins wrote: >>> >>>> I suppose I should ask you to try reverting this 0d1c81edc61e alone >>>> from 6.5-rc1: the consistency of your bisection implies that it will >>>> "fix" the issues, and it is a commit which we could drop. It makes >>>> me a little nervous, applying userspace-pagetable validation to kernel >>>> pagetables, so I don't want to drop it; and it would really be cargo- >>>> culting to drop it without understanding. But we could drop it. >>> >>> I did look at that, it doesn't revert cleanly by itself. ... >> >> Right, that ptep_get() wrapper on the next line came in on top. >> The patch to revert just 0d1c81edc61e is this: > > Still investigating but I'm pretty convinced this is nothing to do with > your commit/series and is just common or garden memory corruption that > just happens to get tickled by your changes. Sorry for the noise. In that case: #regzbot introduced v6.4..v6.5-rc1 #regzbot ignore-activity Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you.
On Tue, Jul 11, 2023 at 06:57:33PM +0100, Mark Brown wrote: > On Tue, Jul 11, 2023 at 09:13:18AM -0700, Hugh Dickins wrote: > > On Tue, 11 Jul 2023, Mark Brown wrote: > > > On Mon, Jul 10, 2023 at 09:34:42PM -0700, Hugh Dickins wrote: > > > > > > > I suppose I should ask you to try reverting this 0d1c81edc61e alone > > > > from 6.5-rc1: the consistency of your bisection implies that it will > > > > "fix" the issues, and it is a commit which we could drop. It makes > > > > me a little nervous, applying userspace-pagetable validation to kernel > > > > pagetables, so I don't want to drop it; and it would really be cargo- > > > > culting to drop it without understanding. But we could drop it. > > > > > > I did look at that, it doesn't revert cleanly by itself. ... > > > > Right, that ptep_get() wrapper on the next line came in on top. > > The patch to revert just 0d1c81edc61e is this: > > Still investigating but I'm pretty convinced this is nothing to do with > your commit/series and is just common or garden memory corruption that > just happens to get tickled by your changes. Sorry for the noise. Did you get to the bottom of this? If not, do you have a reliable way to reproduce the problem? I don't like the sound of memory corruption :( Will
On Thu, Jul 20, 2023 at 11:32:28AM +0100, Will Deacon wrote: > On Tue, Jul 11, 2023 at 06:57:33PM +0100, Mark Brown wrote: > > Still investigating but I'm pretty convinced this is nothing to do with > > your commit/series and is just common or garden memory corruption that > > just happens to get tickled by your changes. Sorry for the noise. > Did you get to the bottom of this? If not, do you have a reliable way to > reproduce the problem? I don't like the sound of memory corruption :( Not to the bottom of it, but getting there - I isolated the issue to something in the unregistration path for thermal zones but didn't manage to figure out exactly what. There was some indication it might be a use after free but I'm not convinced. I have a reliable way to reproduce this if you have a pine64plus, it also shows up a lot on the Libretech Tritium but not quite so reliably as pine64plus since Hugh's changes. Equally pine64plus was rock solid until those so there's some timing/environment thing going on which makes the issue manifest obviously, I expect you should be able to trigger the issue by unregistering a thermal driver but the effects might not be visible. There is a change on the list to make the Allwinner SoCs not trigger the issue during boot (their thermal driver refuses to register if any one zone fails but most of their SoCs have multiple thermal zones with only one fully described) but it needs fixing either way.
On 20.07.23 14:06, Mark Brown wrote: > On Thu, Jul 20, 2023 at 11:32:28AM +0100, Will Deacon wrote: >> On Tue, Jul 11, 2023 at 06:57:33PM +0100, Mark Brown wrote: > >>> Still investigating but I'm pretty convinced this is nothing to do with >>> your commit/series and is just common or garden memory corruption that >>> just happens to get tickled by your changes. Sorry for the noise. > >> Did you get to the bottom of this? If not, do you have a reliable way to >> reproduce the problem? I don't like the sound of memory corruption :( > > Not to the bottom of it, but getting there - I isolated the issue to > something in the unregistration path for thermal zones but didn't manage > to figure out exactly what. Hi Mark, just wondering did anything come out of this and is this still happening? I'm just wondering, as I still have this on my list of tracked regressions. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot poke > There was some indication it might be a use > after free but I'm not convinced. > > I have a reliable way to reproduce this if you have a pine64plus, it > also shows up a lot on the Libretech Tritium but not quite so reliably > as pine64plus since Hugh's changes. Equally pine64plus was rock solid > until those so there's some timing/environment thing going on which > makes the issue manifest obviously, I expect you should be able to > trigger the issue by unregistering a thermal driver but the effects > might not be visible. > > There is a change on the list to make the Allwinner SoCs not trigger the > issue during boot (their thermal driver refuses to register if any one > zone fails but most of their SoCs have multiple thermal zones with only > one fully described) but it needs fixing either way.
On Tue, Aug 08, 2023 at 07:52:43AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > Hi Mark, just wondering did anything come out of this and is this still > happening? I'm just wondering, as I still have this on my list of > tracked regressions. It's fixed.
On 08.08.23 13:09, Mark Brown wrote: > On Tue, Aug 08, 2023 at 07:52:43AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > >> Hi Mark, just wondering did anything come out of this and is this still >> happening? I'm just wondering, as I still have this on my list of >> tracked regressions. > > It's fixed. In that case: #regzbot resolve: fixed according to reporter #regzbot ignore-activity Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you.
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 9683573f1225..741722d247d5 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -703,11 +703,10 @@ struct page *vmalloc_to_page(const void *vmalloc_addr) if (WARN_ON_ONCE(pmd_bad(*pmd))) return NULL; - ptep = pte_offset_map(pmd, addr); + ptep = pte_offset_kernel(pmd, addr); pte = *ptep; if (pte_present(pte)) page = pte_page(pte); - pte_unmap(ptep); return page; }