Skip to content

Commit fdcbb1b

Browse files
committed
Merge branch 'nocache-cleanup'
This series cleans up some of the special user copy functions naming and semantics. In particular, get rid of the (very traditional) double underscore names and behavior: the whole "optimize away the range check" model has been largely excised from the other user accessors because it's so subtle and can be unsafe, but also because it's just not a relevant optimization any more. To do that, a couple of drivers that misused the "user" copies as kernel copies in order to get non-temporal stores had to be fixed up, but that kind of code should never have been allowed anyway. The x86-only "nocache" version was also renamed to more accurately reflect what it actually does. This was all done because I looked at this code due to a report by Jann Horn, and I just couldn't stand the inconsistent naming, the horrible semantics, and the random misuse of these functions. This code should probably be cleaned up further, but it's at least slightly closer to normal semantics. I had a more intrusive series that went even further in trying to normalize the semantics, but that ended up hitting so many other inconsistencies between different architectures in this area (eg 'size_t' vs 'unsigned long' vs 'int' as size arguments, and various iovec check differences that Vasily Gorbik pointed out) that I ended up with this more limited version that fixed the worst of the issues. Reported-by: Jann Horn <jannh@google.com> Tested-by: Will Deacon <will@kernel.org> Link: https://lore.kernel.org/all/CAHk-=wgg1QVWNWG-UCFo1hx0zqrPnB3qhPzUTrWNft+MtXQXig@mail.gmail.com/ * nocache-cleanup: x86-64/arm64/powerpc: clean up and rename __copy_from_user_flushcache x86: rename and clean up __copy_from_user_inatomic_nocache() x86-64: rename misleadingly named '__copy_user_nocache()' function
2 parents 028ef9c + 809b997 commit fdcbb1b

16 files changed

Lines changed: 53 additions & 52 deletions

File tree

arch/arm64/include/asm/uaccess.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ extern __must_check long strnlen_user(const char __user *str, long n);
478478
#ifdef CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE
479479
extern unsigned long __must_check __copy_user_flushcache(void *to, const void __user *from, unsigned long n);
480480

481-
static inline int __copy_from_user_flushcache(void *dst, const void __user *src, unsigned size)
481+
static inline size_t copy_from_user_flushcache(void *dst, const void __user *src, size_t size)
482482
{
483483
kasan_check_write(dst, size);
484484
return __copy_user_flushcache(dst, __uaccess_mask_ptr(src), size);

arch/powerpc/include/asm/uaccess.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,8 +434,7 @@ copy_mc_to_user(void __user *to, const void *from, unsigned long n)
434434
}
435435
#endif
436436

437-
extern long __copy_from_user_flushcache(void *dst, const void __user *src,
438-
unsigned size);
437+
extern size_t copy_from_user_flushcache(void *dst, const void __user *src, size_t size);
439438

440439
static __must_check __always_inline bool __user_access_begin(const void __user *ptr, size_t len,
441440
unsigned long dir)

arch/powerpc/lib/pmem.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,16 @@ EXPORT_SYMBOL_GPL(arch_invalidate_pmem);
6666
/*
6767
* CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE symbols
6868
*/
69-
long __copy_from_user_flushcache(void *dest, const void __user *src,
70-
unsigned size)
69+
size_t copy_from_user_flushcache(void *dest, const void __user *src,
70+
size_t size)
7171
{
72-
unsigned long copied, start = (unsigned long) dest;
72+
unsigned long not_copied, start = (unsigned long) dest;
7373

74-
copied = __copy_from_user(dest, src, size);
74+
src = mask_user_address(src);
75+
not_copied = __copy_from_user(dest, src, size);
7576
clean_pmem_range(start, start + size);
7677

77-
return copied;
78+
return not_copied;
7879
}
7980

8081
void memcpy_flushcache(void *dest, const void *src, size_t size)

arch/x86/include/asm/uaccess.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ extern struct movsl_mask {
507507
} ____cacheline_aligned_in_smp movsl_mask;
508508
#endif
509509

510-
#define ARCH_HAS_NOCACHE_UACCESS 1
510+
#define ARCH_HAS_NONTEMPORAL_UACCESS 1
511511

512512
/*
513513
* The "unsafe" user accesses aren't really "unsafe", but the naming

arch/x86/include/asm/uaccess_32.h

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,7 @@ raw_copy_from_user(void *to, const void __user *from, unsigned long n)
2626
return __copy_user_ll(to, (__force const void *)from, n);
2727
}
2828

29-
static __always_inline unsigned long
30-
__copy_from_user_inatomic_nocache(void *to, const void __user *from,
31-
unsigned long n)
32-
{
33-
return __copy_from_user_ll_nocache_nozero(to, from, n);
34-
}
35-
29+
unsigned long __must_check copy_from_user_inatomic_nontemporal(void *, const void __user *, unsigned long n);
3630
unsigned long __must_check clear_user(void __user *mem, unsigned long len);
3731
unsigned long __must_check __clear_user(void __user *mem, unsigned long len);
3832

arch/x86/include/asm/uaccess_64.h

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,26 +147,28 @@ raw_copy_to_user(void __user *dst, const void *src, unsigned long size)
147147
return copy_user_generic((__force void *)dst, src, size);
148148
}
149149

150-
extern long __copy_user_nocache(void *dst, const void __user *src, unsigned size);
151-
extern long __copy_user_flushcache(void *dst, const void __user *src, unsigned size);
150+
#define copy_to_nontemporal copy_to_nontemporal
151+
extern size_t copy_to_nontemporal(void *dst, const void *src, size_t size);
152+
extern size_t copy_user_flushcache(void *dst, const void __user *src, size_t size);
152153

153154
static inline int
154-
__copy_from_user_inatomic_nocache(void *dst, const void __user *src,
155+
copy_from_user_inatomic_nontemporal(void *dst, const void __user *src,
155156
unsigned size)
156157
{
157158
long ret;
158159
kasan_check_write(dst, size);
160+
src = mask_user_address(src);
159161
stac();
160-
ret = __copy_user_nocache(dst, src, size);
162+
ret = copy_to_nontemporal(dst, (__force const void *)src, size);
161163
clac();
162164
return ret;
163165
}
164166

165-
static inline int
166-
__copy_from_user_flushcache(void *dst, const void __user *src, unsigned size)
167+
static inline size_t
168+
copy_from_user_flushcache(void *dst, const void __user *src, size_t size)
167169
{
168170
kasan_check_write(dst, size);
169-
return __copy_user_flushcache(dst, src, size);
171+
return copy_user_flushcache(dst, src, size);
170172
}
171173

172174
/*

arch/x86/lib/copy_user_uncached_64.S

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
* Output:
2828
* rax uncopied bytes or 0 if successful.
2929
*/
30-
SYM_FUNC_START(__copy_user_nocache)
30+
SYM_FUNC_START(copy_to_nontemporal)
3131
ANNOTATE_NOENDBR
3232
/* If destination is not 7-byte aligned, we'll have to align it */
3333
testb $7,%dil
@@ -240,5 +240,5 @@ _ASM_EXTABLE_UA(95b, .Ldone)
240240
_ASM_EXTABLE_UA(52b, .Ldone0)
241241
_ASM_EXTABLE_UA(53b, .Ldone0)
242242

243-
SYM_FUNC_END(__copy_user_nocache)
244-
EXPORT_SYMBOL(__copy_user_nocache)
243+
SYM_FUNC_END(copy_to_nontemporal)
244+
EXPORT_SYMBOL(copy_to_nontemporal)

arch/x86/lib/usercopy_32.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,11 @@ unsigned long __copy_user_ll(void *to, const void *from, unsigned long n)
322322
}
323323
EXPORT_SYMBOL(__copy_user_ll);
324324

325-
unsigned long __copy_from_user_ll_nocache_nozero(void *to, const void __user *from,
325+
unsigned long copy_from_user_inatomic_nontemporal(void *to, const void __user *from,
326326
unsigned long n)
327327
{
328-
__uaccess_begin_nospec();
328+
if (!user_access_begin(from, n))
329+
return n;
329330
#ifdef CONFIG_X86_INTEL_USERCOPY
330331
if (n > 64 && static_cpu_has(X86_FEATURE_XMM2))
331332
n = __copy_user_intel_nocache(to, from, n);
@@ -334,7 +335,7 @@ unsigned long __copy_from_user_ll_nocache_nozero(void *to, const void __user *fr
334335
#else
335336
__copy_user(to, from, n);
336337
#endif
337-
__uaccess_end();
338+
user_access_end();
338339
return n;
339340
}
340-
EXPORT_SYMBOL(__copy_from_user_ll_nocache_nozero);
341+
EXPORT_SYMBOL(copy_from_user_inatomic_nontemporal);

arch/x86/lib/usercopy_64.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,17 @@ void arch_wb_cache_pmem(void *addr, size_t size)
4343
}
4444
EXPORT_SYMBOL_GPL(arch_wb_cache_pmem);
4545

46-
long __copy_user_flushcache(void *dst, const void __user *src, unsigned size)
46+
size_t copy_user_flushcache(void *dst, const void __user *src, size_t size)
4747
{
4848
unsigned long flushed, dest = (unsigned long) dst;
49-
long rc;
49+
unsigned long rc;
5050

51-
stac();
52-
rc = __copy_user_nocache(dst, src, size);
53-
clac();
51+
src = masked_user_access_begin(src);
52+
rc = copy_to_nontemporal(dst, (__force const void *)src, size);
53+
user_access_end();
5454

5555
/*
56-
* __copy_user_nocache() uses non-temporal stores for the bulk
56+
* copy_to_nontemporal() uses non-temporal stores for the bulk
5757
* of the transfer, but we need to manually flush if the
5858
* transfer is unaligned. A cached memory copy is used when
5959
* destination or size is not naturally aligned. That is:

drivers/gpu/drm/i915/i915_gem.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ ggtt_write(struct io_mapping *mapping,
520520

521521
/* We can use the cpu mem copy function because this is X86. */
522522
vaddr = io_mapping_map_atomic_wc(mapping, base);
523-
unwritten = __copy_from_user_inatomic_nocache((void __force *)vaddr + offset,
523+
unwritten = copy_from_user_inatomic_nontemporal((void __force *)vaddr + offset,
524524
user_data, length);
525525
io_mapping_unmap_atomic(vaddr);
526526
if (unwritten) {

0 commit comments

Comments
 (0)