Skip to content

Commit a85748e

Browse files
djbwgregkh
authored andcommitted
x86, powerpc: Rename memcpy_mcsafe() to copy_mc_to_{user, kernel}()
commit ec6347b upstream. In reaction to a proposal to introduce a memcpy_mcsafe_fast() implementation Linus points out that memcpy_mcsafe() is poorly named relative to communicating the scope of the interface. Specifically what addresses are valid to pass as source, destination, and what faults / exceptions are handled. Of particular concern is that even though x86 might be able to handle the semantics of copy_mc_to_user() with its common copy_user_generic() implementation other archs likely need / want an explicit path for this case: On Fri, May 1, 2020 at 11:28 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Thu, Apr 30, 2020 at 6:21 PM Dan Williams <dan.j.williams@intel.com> wrote: > > > > However now I see that copy_user_generic() works for the wrong reason. > > It works because the exception on the source address due to poison > > looks no different than a write fault on the user address to the > > caller, it's still just a short copy. So it makes copy_to_user() work > > for the wrong reason relative to the name. > > Right. > > And it won't work that way on other architectures. On x86, we have a > generic function that can take faults on either side, and we use it > for both cases (and for the "in_user" case too), but that's an > artifact of the architecture oddity. > > In fact, it's probably wrong even on x86 - because it can hide bugs - > but writing those things is painful enough that everybody prefers > having just one function. Replace a single top-level memcpy_mcsafe() with either copy_mc_to_user(), or copy_mc_to_kernel(). Introduce an x86 copy_mc_fragile() name as the rename for the low-level x86 implementation formerly named memcpy_mcsafe(). It is used as the slow / careful backend that is supplanted by a fast copy_mc_generic() in a follow-on patch. One side-effect of this reorganization is that separating copy_mc_64.S to its own file means that perf no longer needs to track dependencies for its memcpy_64.S benchmarks. [ bp: Massage a bit. ] Signed-off-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Borislav Petkov <bp@suse.de> Reviewed-by: Tony Luck <tony.luck@intel.com> Acked-by: Michael Ellerman <mpe@ellerman.id.au> Cc: <stable@vger.kernel.org> Link: http://lore.kernel.org/r/CAHk-=wjSqtXAqfUJxFtWNwmguFASTgB0dz1dT3V-78Quiezqbg@mail.gmail.com Link: https://lkml.kernel.org/r/160195561680.2163339.11574962055305783722.stgit@dwillia2-desk3.amr.corp.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent ff0028e commit a85748e

37 files changed

Lines changed: 674 additions & 530 deletions

File tree

arch/powerpc/Kconfig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ config PPC
135135
select ARCH_HAS_STRICT_KERNEL_RWX if (PPC32 && !HIBERNATION)
136136
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
137137
select ARCH_HAS_UACCESS_FLUSHCACHE
138-
select ARCH_HAS_UACCESS_MCSAFE if PPC64
138+
select ARCH_HAS_COPY_MC if PPC64
139139
select ARCH_HAS_UBSAN_SANITIZE_ALL
140140
select ARCH_HAVE_NMI_SAFE_CMPXCHG
141141
select ARCH_KEEP_MEMBLOCK

arch/powerpc/include/asm/string.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ void *__memmove(void *to, const void *from, __kernel_size_t n);
5353
#ifndef CONFIG_KASAN
5454
#define __HAVE_ARCH_MEMSET32
5555
#define __HAVE_ARCH_MEMSET64
56-
#define __HAVE_ARCH_MEMCPY_MCSAFE
5756

58-
extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz);
5957
extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t);
6058
extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t);
6159
extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t);

arch/powerpc/include/asm/uaccess.h

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,32 @@ do { \
435435
extern unsigned long __copy_tofrom_user(void __user *to,
436436
const void __user *from, unsigned long size);
437437

438+
#ifdef CONFIG_ARCH_HAS_COPY_MC
439+
unsigned long __must_check
440+
copy_mc_generic(void *to, const void *from, unsigned long size);
441+
442+
static inline unsigned long __must_check
443+
copy_mc_to_kernel(void *to, const void *from, unsigned long size)
444+
{
445+
return copy_mc_generic(to, from, size);
446+
}
447+
#define copy_mc_to_kernel copy_mc_to_kernel
448+
449+
static inline unsigned long __must_check
450+
copy_mc_to_user(void __user *to, const void *from, unsigned long n)
451+
{
452+
if (likely(check_copy_size(from, n, true))) {
453+
if (access_ok(to, n)) {
454+
allow_write_to_user(to, n);
455+
n = copy_mc_generic((void *)to, from, n);
456+
prevent_write_to_user(to, n);
457+
}
458+
}
459+
460+
return n;
461+
}
462+
#endif
463+
438464
#ifdef __powerpc64__
439465
static inline unsigned long
440466
raw_copy_in_user(void __user *to, const void __user *from, unsigned long n)
@@ -523,20 +549,6 @@ raw_copy_to_user(void __user *to, const void *from, unsigned long n)
523549
return ret;
524550
}
525551

526-
static __always_inline unsigned long __must_check
527-
copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n)
528-
{
529-
if (likely(check_copy_size(from, n, true))) {
530-
if (access_ok(to, n)) {
531-
allow_write_to_user(to, n);
532-
n = memcpy_mcsafe((void *)to, from, n);
533-
prevent_write_to_user(to, n);
534-
}
535-
}
536-
537-
return n;
538-
}
539-
540552
unsigned long __arch_clear_user(void __user *addr, unsigned long size);
541553

542554
static inline unsigned long clear_user(void __user *addr, unsigned long size)

arch/powerpc/lib/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \
3939
memcpy_power7.o
4040

4141
obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \
42-
memcpy_64.o memcpy_mcsafe_64.o
42+
memcpy_64.o copy_mc_64.o
4343

4444
ifndef CONFIG_PPC_QUEUED_SPINLOCKS
4545
obj64-$(CONFIG_SMP) += locks.o
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ err3; stb r0,0(r3)
5050
blr
5151

5252

53-
_GLOBAL(memcpy_mcsafe)
53+
_GLOBAL(copy_mc_generic)
5454
mr r7,r5
5555
cmpldi r5,16
5656
blt .Lshort_copy
@@ -239,4 +239,4 @@ err1; stb r0,0(r3)
239239
15: li r3,0
240240
blr
241241

242-
EXPORT_SYMBOL_GPL(memcpy_mcsafe);
242+
EXPORT_SYMBOL_GPL(copy_mc_generic);

arch/x86/Kconfig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ config X86
7575
select ARCH_HAS_PTE_DEVMAP if X86_64
7676
select ARCH_HAS_PTE_SPECIAL
7777
select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64
78-
select ARCH_HAS_UACCESS_MCSAFE if X86_64 && X86_MCE
78+
select ARCH_HAS_COPY_MC if X86_64
7979
select ARCH_HAS_SET_MEMORY
8080
select ARCH_HAS_SET_DIRECT_MAP
8181
select ARCH_HAS_STRICT_KERNEL_RWX

arch/x86/Kconfig.debug

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ config EARLY_PRINTK_USB_XDBC
6262
You should normally say N here, unless you want to debug early
6363
crashes or need a very simple printk logging facility.
6464

65-
config MCSAFE_TEST
65+
config COPY_MC_TEST
6666
def_bool n
6767

6868
config EFI_PGT_DUMP
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/* SPDX-License-Identifier: GPL-2.0 */
2+
#ifndef _COPY_MC_TEST_H_
3+
#define _COPY_MC_TEST_H_
4+
5+
#ifndef __ASSEMBLY__
6+
#ifdef CONFIG_COPY_MC_TEST
7+
extern unsigned long copy_mc_test_src;
8+
extern unsigned long copy_mc_test_dst;
9+
10+
static inline void copy_mc_inject_src(void *addr)
11+
{
12+
if (addr)
13+
copy_mc_test_src = (unsigned long) addr;
14+
else
15+
copy_mc_test_src = ~0UL;
16+
}
17+
18+
static inline void copy_mc_inject_dst(void *addr)
19+
{
20+
if (addr)
21+
copy_mc_test_dst = (unsigned long) addr;
22+
else
23+
copy_mc_test_dst = ~0UL;
24+
}
25+
#else /* CONFIG_COPY_MC_TEST */
26+
static inline void copy_mc_inject_src(void *addr)
27+
{
28+
}
29+
30+
static inline void copy_mc_inject_dst(void *addr)
31+
{
32+
}
33+
#endif /* CONFIG_COPY_MC_TEST */
34+
35+
#else /* __ASSEMBLY__ */
36+
#include <asm/export.h>
37+
38+
#ifdef CONFIG_COPY_MC_TEST
39+
.macro COPY_MC_TEST_CTL
40+
.pushsection .data
41+
.align 8
42+
.globl copy_mc_test_src
43+
copy_mc_test_src:
44+
.quad 0
45+
EXPORT_SYMBOL_GPL(copy_mc_test_src)
46+
.globl copy_mc_test_dst
47+
copy_mc_test_dst:
48+
.quad 0
49+
EXPORT_SYMBOL_GPL(copy_mc_test_dst)
50+
.popsection
51+
.endm
52+
53+
.macro COPY_MC_TEST_SRC reg count target
54+
leaq \count(\reg), %r9
55+
cmp copy_mc_test_src, %r9
56+
ja \target
57+
.endm
58+
59+
.macro COPY_MC_TEST_DST reg count target
60+
leaq \count(\reg), %r9
61+
cmp copy_mc_test_dst, %r9
62+
ja \target
63+
.endm
64+
#else
65+
.macro COPY_MC_TEST_CTL
66+
.endm
67+
68+
.macro COPY_MC_TEST_SRC reg count target
69+
.endm
70+
71+
.macro COPY_MC_TEST_DST reg count target
72+
.endm
73+
#endif /* CONFIG_COPY_MC_TEST */
74+
#endif /* __ASSEMBLY__ */
75+
#endif /* _COPY_MC_TEST_H_ */

arch/x86/include/asm/mce.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,15 @@ extern void mce_unregister_decode_chain(struct notifier_block *nb);
174174

175175
extern int mce_p5_enabled;
176176

177+
#ifdef CONFIG_ARCH_HAS_COPY_MC
178+
extern void enable_copy_mc_fragile(void);
179+
unsigned long __must_check copy_mc_fragile(void *dst, const void *src, unsigned cnt);
180+
#else
181+
static inline void enable_copy_mc_fragile(void)
182+
{
183+
}
184+
#endif
185+
177186
#ifdef CONFIG_X86_MCE
178187
int mcheck_init(void);
179188
void mcheck_cpu_init(struct cpuinfo_x86 *c);

arch/x86/include/asm/mcsafe_test.h

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)