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

Reply via email to