* [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local()
@ 2021-07-07 8:30 Mark Rutland
2021-07-07 8:53 ` [tip: locking/urgent] locking/atomic: sparc: Fix arch_cmpxchg64_local() tip-bot2 for Mark Rutland
2021-07-07 20:56 ` [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local() David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Mark Rutland @ 2021-07-07 8:30 UTC (permalink / raw)
To: linux-kernel, peterz, davem; +Cc: Mark Rutland, Ingo Molnar, sparclinux
Anatoly reports that since commit:
ff5b4f1ed580c59d ("locking/atomic: sparc: move to ARCH_ATOMIC")
... it's possible to reliably trigger an oops by running:
stress-ng -v --mmap 1 -t 30s
... which results in a NULL pointer dereference in
__split_huge_pmd_locked().
The underlying problem is that commit ff5b4f1ed580c59d left
arch_cmpxchg64_local() defined in terms of cmpxchg_local() rather than
arch_cmpxchg_local(). In <asm-generic/atomic-instrumented.h> we wrap
these with macros which use identically-named variables. When
cmpxchg_local() nests inside cmpxchg64_local(), this casues it to use an
unitialized variable as the pointer, which can be NULL.
This can also be seen in pmdp_establish(), where the compiler can
generate the pointer with a `clr` instruction:
0000000000000360 <pmdp_establish>:
360: 9d e3 bf 50 save %sp, -176, %sp
364: fa 5e 80 00 ldx [ %i2 ], %i5
368: 82 10 00 1b mov %i3, %g1
36c: 84 10 20 00 clr %g2
370: c3 f0 90 1d casx [ %g2 ], %i5, %g1
374: 80 a7 40 01 cmp %i5, %g1
378: 32 6f ff fc bne,a %xcc, 368 <pmdp_establish+0x8>
37c: fa 5e 80 00 ldx [ %i2 ], %i5
380: d0 5e 20 40 ldx [ %i0 + 0x40 ], %o0
384: 96 10 00 1b mov %i3, %o3
388: 94 10 00 1d mov %i5, %o2
38c: 92 10 00 19 mov %i1, %o1
390: 7f ff ff 84 call 1a0 <__set_pmd_acct>
394: b0 10 00 1d mov %i5, %i0
398: 81 cf e0 08 return %i7 + 8
39c: 01 00 00 00 nop
This patch fixes the problem by defining arch_cmpxchg64_local() in terms
of arch_cmpxchg_local(), avoiding potential shadowing, and resulting in
working cmpxchg64_local() and variants, e.g.
0000000000000360 <pmdp_establish>:
360: 9d e3 bf 50 save %sp, -176, %sp
364: fa 5e 80 00 ldx [ %i2 ], %i5
368: 82 10 00 1b mov %i3, %g1
36c: c3 f6 90 1d casx [ %i2 ], %i5, %g1
370: 80 a7 40 01 cmp %i5, %g1
374: 32 6f ff fd bne,a %xcc, 368 <pmdp_establish+0x8>
378: fa 5e 80 00 ldx [ %i2 ], %i5
37c: d0 5e 20 40 ldx [ %i0 + 0x40 ], %o0
380: 96 10 00 1b mov %i3, %o3
384: 94 10 00 1d mov %i5, %o2
388: 92 10 00 19 mov %i1, %o1
38c: 7f ff ff 85 call 1a0 <__set_pmd_acct>
390: b0 10 00 1d mov %i5, %i0
394: 81 cf e0 08 return %i7 + 8
398: 01 00 00 00 nop
39c: 01 00 00 00 nop
Link: https://lore.kernel.org/r/CADxRZqzcrnSMzy50T+kWb_mQVguWDCMu6RoXsCc+-fNDPYXbaw@mail.gmail.com
Fixes: ff5b4f1ed580c59d ("locking/atomic: sparc: move to ARCH_ATOMIC")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Anatoly Pugachev <matorola@gmail.com>
Tested-by: Anatoly Pugachev <matorola@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@lists.infradead.org>
Cc: sparclinux@vger.kernel.org
---
arch/sparc/include/asm/cmpxchg_64.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Peter, David, could one of you please apply this? It's an urgent fix for
fallout from the ARCH_ATOMIC conversion, and it'd be good to fix before -rc1.
Thanks,
Mark.
diff --git a/arch/sparc/include/asm/cmpxchg_64.h b/arch/sparc/include/asm/cmpxchg_64.h
index 8c39a9981187..12d00a42c0a3 100644
--- a/arch/sparc/include/asm/cmpxchg_64.h
+++ b/arch/sparc/include/asm/cmpxchg_64.h
@@ -201,7 +201,7 @@ static inline unsigned long __cmpxchg_local(volatile void *ptr,
#define arch_cmpxchg64_local(ptr, o, n) \
({ \
BUILD_BUG_ON(sizeof(*(ptr)) != 8); \
- cmpxchg_local((ptr), (o), (n)); \
+ arch_cmpxchg_local((ptr), (o), (n)); \
})
#define arch_cmpxchg64(ptr, o, n) arch_cmpxchg64_local((ptr), (o), (n))
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [tip: locking/urgent] locking/atomic: sparc: Fix arch_cmpxchg64_local()
2021-07-07 8:30 [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local() Mark Rutland
@ 2021-07-07 8:53 ` tip-bot2 for Mark Rutland
2021-07-07 20:56 ` [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local() David Miller
1 sibling, 0 replies; 3+ messages in thread
From: tip-bot2 for Mark Rutland @ 2021-07-07 8:53 UTC (permalink / raw)
To: linux-tip-commits
Cc: Anatoly Pugachev, Mark Rutland, Ingo Molnar, x86, linux-kernel
The following commit has been merged into the locking/urgent branch of tip:
Commit-ID: 7e1088760cfe0bb1fdb1f0bd155bfd52f080683a
Gitweb: https://git.kernel.org/tip/7e1088760cfe0bb1fdb1f0bd155bfd52f080683a
Author: Mark Rutland <mark.rutland@arm.com>
AuthorDate: Wed, 07 Jul 2021 09:30:32 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 07 Jul 2021 10:47:21 +02:00
locking/atomic: sparc: Fix arch_cmpxchg64_local()
Anatoly reports that since commit:
ff5b4f1ed580c59d ("locking/atomic: sparc: move to ARCH_ATOMIC")
... it's possible to reliably trigger an oops by running:
stress-ng -v --mmap 1 -t 30s
... which results in a NULL pointer dereference in
__split_huge_pmd_locked().
The underlying problem is that commit ff5b4f1ed580c59d left
arch_cmpxchg64_local() defined in terms of cmpxchg_local() rather than
arch_cmpxchg_local(). In <asm-generic/atomic-instrumented.h> we wrap
these with macros which use identically-named variables. When
cmpxchg_local() nests inside cmpxchg64_local(), this casues it to use an
unitialized variable as the pointer, which can be NULL.
This can also be seen in pmdp_establish(), where the compiler can
generate the pointer with a `clr` instruction:
0000000000000360 <pmdp_establish>:
360: 9d e3 bf 50 save %sp, -176, %sp
364: fa 5e 80 00 ldx [ %i2 ], %i5
368: 82 10 00 1b mov %i3, %g1
36c: 84 10 20 00 clr %g2
370: c3 f0 90 1d casx [ %g2 ], %i5, %g1
374: 80 a7 40 01 cmp %i5, %g1
378: 32 6f ff fc bne,a %xcc, 368 <pmdp_establish+0x8>
37c: fa 5e 80 00 ldx [ %i2 ], %i5
380: d0 5e 20 40 ldx [ %i0 + 0x40 ], %o0
384: 96 10 00 1b mov %i3, %o3
388: 94 10 00 1d mov %i5, %o2
38c: 92 10 00 19 mov %i1, %o1
390: 7f ff ff 84 call 1a0 <__set_pmd_acct>
394: b0 10 00 1d mov %i5, %i0
398: 81 cf e0 08 return %i7 + 8
39c: 01 00 00 00 nop
This patch fixes the problem by defining arch_cmpxchg64_local() in terms
of arch_cmpxchg_local(), avoiding potential shadowing, and resulting in
working cmpxchg64_local() and variants, e.g.
0000000000000360 <pmdp_establish>:
360: 9d e3 bf 50 save %sp, -176, %sp
364: fa 5e 80 00 ldx [ %i2 ], %i5
368: 82 10 00 1b mov %i3, %g1
36c: c3 f6 90 1d casx [ %i2 ], %i5, %g1
370: 80 a7 40 01 cmp %i5, %g1
374: 32 6f ff fd bne,a %xcc, 368 <pmdp_establish+0x8>
378: fa 5e 80 00 ldx [ %i2 ], %i5
37c: d0 5e 20 40 ldx [ %i0 + 0x40 ], %o0
380: 96 10 00 1b mov %i3, %o3
384: 94 10 00 1d mov %i5, %o2
388: 92 10 00 19 mov %i1, %o1
38c: 7f ff ff 85 call 1a0 <__set_pmd_acct>
390: b0 10 00 1d mov %i5, %i0
394: 81 cf e0 08 return %i7 + 8
398: 01 00 00 00 nop
39c: 01 00 00 00 nop
Fixes: ff5b4f1ed580c59d ("locking/atomic: sparc: move to ARCH_ATOMIC")
Reported-by: Anatoly Pugachev <matorola@gmail.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Tested-by: Anatoly Pugachev <matorola@gmail.com>
Link: https://lore.kernel.org/r/20210707083032.567-1-mark.rutland@arm.com
---
arch/sparc/include/asm/cmpxchg_64.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sparc/include/asm/cmpxchg_64.h b/arch/sparc/include/asm/cmpxchg_64.h
index 8c39a99..12d00a4 100644
--- a/arch/sparc/include/asm/cmpxchg_64.h
+++ b/arch/sparc/include/asm/cmpxchg_64.h
@@ -201,7 +201,7 @@ static inline unsigned long __cmpxchg_local(volatile void *ptr,
#define arch_cmpxchg64_local(ptr, o, n) \
({ \
BUILD_BUG_ON(sizeof(*(ptr)) != 8); \
- cmpxchg_local((ptr), (o), (n)); \
+ arch_cmpxchg_local((ptr), (o), (n)); \
})
#define arch_cmpxchg64(ptr, o, n) arch_cmpxchg64_local((ptr), (o), (n))
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local()
2021-07-07 8:30 [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local() Mark Rutland
2021-07-07 8:53 ` [tip: locking/urgent] locking/atomic: sparc: Fix arch_cmpxchg64_local() tip-bot2 for Mark Rutland
@ 2021-07-07 20:56 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2021-07-07 20:56 UTC (permalink / raw)
To: mark.rutland; +Cc: linux-kernel, peterz, mingo, sparclinux
From: Mark Rutland <mark.rutland@arm.com>
Date: Wed, 7 Jul 2021 09:30:32 +0100
> Peter, David, could one of you please apply this? It's an urgent fix for
> fallout from the ARCH_ATOMIC conversion, and it'd be good to fix before -rc1.
Peter please take this as I am backlogged.
Thank you.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-07 20:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 8:30 [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local() Mark Rutland
2021-07-07 8:53 ` [tip: locking/urgent] locking/atomic: sparc: Fix arch_cmpxchg64_local() tip-bot2 for Mark Rutland
2021-07-07 20:56 ` [PATCH] locking/atomic: sparc: fix arch_cmpxchg64_local() David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).