[v2,12/32] mm/vmalloc: vmalloc_to_page() use pte_offset_kernel()

  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>
---
 mm/vmalloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
  

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.