Fix __user annotations in various places across the x86 tree: - cast to wrong pointer type in __user_atomic_cmpxchg_inatomic() - generic_load_microcode() deals with a pointer that can be either a kernel pointer or a user pointer; change the code to pass it around as a __user pointer, and add explicit casts to convert between __user and __kernel - save_xstate_epilog() has missing __user in explicit casts - setup_sigcontext() and x32_setup_rt_frame() rely on the cast performed by put_user_ex() on its first argument, but sparse requires __force for casting __user pointers to unsigned long - xen_hvm_config() has missing __user
This patch removes all sparse warnings about the asn:1 address space (__user) in arch/x86/ for my kernel config. Signed-off-by: Jann Horn <[email protected]> --- This patch requires the previous one, "[PATCH 1/2] kernel.h: use parentheses around argument in u64_to_user_ptr()", otherwise xen_hvm_config() breaks. Can we take both together through the x86 tree, or does the first one have to go through akpm's tree? arch/x86/include/asm/uaccess.h | 3 +-- arch/x86/include/asm/uaccess_64.h | 2 +- arch/x86/kernel/cpu/microcode/intel.c | 20 ++++++++++++-------- arch/x86/kernel/fpu/signal.c | 6 +++--- arch/x86/kernel/signal.c | 4 ++-- arch/x86/kvm/x86.c | 8 ++++---- arch/x86/lib/usercopy_64.c | 4 +++- 7 files changed, 26 insertions(+), 21 deletions(-) diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 1954dd5552a2..a21f2a2f17bf 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -585,7 +585,6 @@ extern void __cmpxchg_wrong_size(void) #define __user_atomic_cmpxchg_inatomic(uval, ptr, old, new, size) \ ({ \ int __ret = 0; \ - __typeof__(ptr) __uval = (uval); \ __typeof__(*(ptr)) __old = (old); \ __typeof__(*(ptr)) __new = (new); \ __uaccess_begin_nospec(); \ @@ -661,7 +660,7 @@ extern void __cmpxchg_wrong_size(void) __cmpxchg_wrong_size(); \ } \ __uaccess_end(); \ - *__uval = __old; \ + *(uval) = __old; \ __ret; \ }) diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h index a9d637bc301d..cbca2cb28939 100644 --- a/arch/x86/include/asm/uaccess_64.h +++ b/arch/x86/include/asm/uaccess_64.h @@ -208,7 +208,7 @@ __copy_from_user_flushcache(void *dst, const void __user *src, unsigned size) } unsigned long -copy_user_handle_tail(char *to, char *from, unsigned len); +copy_user_handle_tail(char __user *to, char __user *from, unsigned len); unsigned long mcsafe_handle_tail(char *to, char *from, unsigned len); diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 16936a24795c..e8ef65c275c7 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -861,11 +861,13 @@ static enum ucode_state apply_microcode_intel(int cpu) return ret; } -static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, - int (*get_ucode_data)(void *, const void *, size_t)) +static enum ucode_state generic_load_microcode(int cpu, + const void __user *data, size_t size, + int (*get_ucode_data)(void *, const void __user *, size_t)) { struct ucode_cpu_info *uci = ucode_cpu_info + cpu; - u8 *ucode_ptr = data, *new_mc = NULL, *mc = NULL; + const u8 __user *ucode_ptr = data; + u8 *new_mc = NULL, *mc = NULL; int new_rev = uci->cpu_sig.rev; unsigned int leftover = size; unsigned int curr_mc_size = 0, new_mc_size = 0; @@ -945,9 +947,10 @@ static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, return ret; } -static int get_ucode_fw(void *to, const void *from, size_t n) +static int get_ucode_fw(void *to, const void __user *from, size_t n) { - memcpy(to, from, n); + /* cast paired with request_microcode_fw() */ + memcpy(to, (const void __force *)from, n); return 0; } @@ -993,7 +996,8 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device, return UCODE_NFOUND; } - ret = generic_load_microcode(cpu, (void *)firmware->data, + /* cast paired with get_ucode_fw() */ + ret = generic_load_microcode(cpu, (void __force __user *)firmware->data, firmware->size, &get_ucode_fw); release_firmware(firmware); @@ -1001,7 +1005,7 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device, return ret; } -static int get_ucode_user(void *to, const void *from, size_t n) +static int get_ucode_user(void *to, const void __user *from, size_t n) { return copy_from_user(to, from, n); } @@ -1012,7 +1016,7 @@ request_microcode_user(int cpu, const void __user *buf, size_t size) if (is_blacklisted(cpu)) return UCODE_NFOUND; - return generic_load_microcode(cpu, (void *)buf, size, &get_ucode_user); + return generic_load_microcode(cpu, buf, size, &get_ucode_user); } static struct microcode_ops microcode_intel_ops = { diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c index f6a1d299627c..55b80de13ea5 100644 --- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -92,13 +92,13 @@ static inline int save_xstate_epilog(void __user *buf, int ia32_frame) return err; err |= __put_user(FP_XSTATE_MAGIC2, - (__u32 *)(buf + fpu_user_xstate_size)); + (__u32 __user *)(buf + fpu_user_xstate_size)); /* * Read the xfeatures which we copied (directly from the cpu or * from the state in task struct) to the user buffers. */ - err |= __get_user(xfeatures, (__u32 *)&x->header.xfeatures); + err |= __get_user(xfeatures, (__u32 __user *)&x->header.xfeatures); /* * For legacy compatible, we always set FP/SSE bits in the bit @@ -113,7 +113,7 @@ static inline int save_xstate_epilog(void __user *buf, int ia32_frame) */ xfeatures |= XFEATURE_MASK_FPSSE; - err |= __put_user(xfeatures, (__u32 *)&x->header.xfeatures); + err |= __put_user(xfeatures, (__u32 __user *)&x->header.xfeatures); return err; } diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c index 08dfd4c1a4f9..e13cd972f9af 100644 --- a/arch/x86/kernel/signal.c +++ b/arch/x86/kernel/signal.c @@ -206,7 +206,7 @@ int setup_sigcontext(struct sigcontext __user *sc, void __user *fpstate, put_user_ex(regs->ss, &sc->ss); #endif /* CONFIG_X86_32 */ - put_user_ex(fpstate, &sc->fpstate); + put_user_ex((unsigned long __force)fpstate, &sc->fpstate); /* non-iBCS2 extensions.. */ put_user_ex(mask, &sc->oldmask); @@ -569,7 +569,7 @@ static int x32_setup_rt_frame(struct ksignal *ksig, restorer = NULL; err |= -EFAULT; } - put_user_ex(restorer, &frame->pretcode); + put_user_ex((unsigned long __force)restorer, &frame->pretcode); } put_user_catch(err); err |= setup_sigcontext(&frame->uc.uc_mcontext, fpstate, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 65e4559eef2f..ca087debefbf 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2317,11 +2317,11 @@ static int set_msr_mce(struct kvm_vcpu *vcpu, struct msr_data *msr_info) static int xen_hvm_config(struct kvm_vcpu *vcpu, u64 data) { struct kvm *kvm = vcpu->kvm; + struct kvm_xen_hvm_config *cfg = &kvm->arch.xen_hvm_config; int lm = is_long_mode(vcpu); - u8 *blob_addr = lm ? (u8 *)(long)kvm->arch.xen_hvm_config.blob_addr_64 - : (u8 *)(long)kvm->arch.xen_hvm_config.blob_addr_32; - u8 blob_size = lm ? kvm->arch.xen_hvm_config.blob_size_64 - : kvm->arch.xen_hvm_config.blob_size_32; + u8 __user *blob_addr = + u64_to_user_ptr(lm ? cfg->blob_addr_64 : cfg->blob_addr_32); + u8 blob_size = lm ? cfg->blob_size_64 : cfg->blob_size_32; u32 page_num = data & ~PAGE_MASK; u64 page_addr = data & PAGE_MASK; u8 *page; diff --git a/arch/x86/lib/usercopy_64.c b/arch/x86/lib/usercopy_64.c index ee42bb0cbeb3..cd8a1802adde 100644 --- a/arch/x86/lib/usercopy_64.c +++ b/arch/x86/lib/usercopy_64.c @@ -58,9 +58,11 @@ EXPORT_SYMBOL(clear_user); * Try to copy last bytes and clear the rest if needed. * Since protection fault in copy_from/to_user is not a normal situation, * it is not necessary to optimize tail handling. + * We don't know which side of the copy is in userspace; we treat both sides as + * user pointers. */ __visible unsigned long -copy_user_handle_tail(char *to, char *from, unsigned len) +copy_user_handle_tail(char __user *to, char __user *from, unsigned len) { for (; len; --len, to++) { char c; -- 2.21.0.392.gf8f6787159e-goog

