s390/maccess: fix semantics of memcpy_real() and its callers
authorAlexander Gordeev <agordeev@linux.ibm.com>
Sat, 29 Jan 2022 08:24:50 +0000 (09:24 +0100)
committerVasily Gorbik <gor@linux.ibm.com>
Wed, 9 Feb 2022 21:56:04 +0000 (22:56 +0100)
There is a confusion with regard to the source address of
memcpy_real() and calling functions. While the declared
type for a source assumes a virtual address, in fact it
always called with physical address of the source.

This confusion led to bugs in copy_oldmem_kernel() and
copy_oldmem_user() functions, where __pa() macro applied
mistakenly to physical addresses. It does not lead to a
real issue, since virtual and physical addresses are
currently the same.

Fix both the bugs and memcpy_real() prototype by making
type of source address consistent to the function name
and the way it actually used.

Reviewed-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
arch/s390/include/asm/os_info.h
arch/s390/include/asm/processor.h
arch/s390/include/asm/uaccess.h
arch/s390/kernel/crash_dump.c
arch/s390/kernel/os_info.c
arch/s390/kernel/smp.c
arch/s390/mm/maccess.c
drivers/s390/char/zcore.c

index 3c89279..147a8d5 100644 (file)
@@ -39,7 +39,7 @@ u32 os_info_csum(struct os_info *os_info);
 
 #ifdef CONFIG_CRASH_DUMP
 void *os_info_old_entry(int nr, unsigned long *size);
-int copy_oldmem_kernel(void *dst, void *src, size_t count);
+int copy_oldmem_kernel(void *dst, unsigned long src, size_t count);
 #else
 static inline void *os_info_old_entry(int nr, unsigned long *size)
 {
index 5581b64..8fd9772 100644 (file)
@@ -317,7 +317,7 @@ extern void (*s390_base_pgm_handler_fn)(void);
 
 #define ARCH_LOW_ADDRESS_LIMIT 0x7fffffffUL
 
-extern int memcpy_real(void *, void *, size_t);
+extern int memcpy_real(void *, unsigned long, size_t);
 extern void memcpy_absolute(void *, void *, size_t);
 
 #define mem_assign_absolute(dest, val) do {                    \
index d74e26b..f14f4ad 100644 (file)
@@ -279,7 +279,7 @@ static inline unsigned long __must_check clear_user(void __user *to, unsigned lo
        return __clear_user(to, n);
 }
 
-int copy_to_user_real(void __user *dest, void *src, unsigned long count);
+int copy_to_user_real(void __user *dest, unsigned long src, unsigned long count);
 void *s390_kernel_write(void *dst, const void *src, size_t size);
 
 #define HAVE_GET_KERNEL_NOFAULT
index a62bee8..69819b7 100644 (file)
@@ -132,28 +132,27 @@ static inline void *load_real_addr(void *addr)
 /*
  * Copy memory of the old, dumped system to a kernel space virtual address
  */
-int copy_oldmem_kernel(void *dst, void *src, size_t count)
+int copy_oldmem_kernel(void *dst, unsigned long src, size_t count)
 {
-       unsigned long from, len;
+       unsigned long len;
        void *ra;
        int rc;
 
        while (count) {
-               from = __pa(src);
-               if (!oldmem_data.start && from < sclp.hsa_size) {
+               if (!oldmem_data.start && src < sclp.hsa_size) {
                        /* Copy from zfcp/nvme dump HSA area */
-                       len = min(count, sclp.hsa_size - from);
-                       rc = memcpy_hsa_kernel(dst, from, len);
+                       len = min(count, sclp.hsa_size - src);
+                       rc = memcpy_hsa_kernel(dst, src, len);
                        if (rc)
                                return rc;
                } else {
                        /* Check for swapped kdump oldmem areas */
-                       if (oldmem_data.start && from - oldmem_data.start < oldmem_data.size) {
-                               from -= oldmem_data.start;
-                               len = min(count, oldmem_data.size - from);
-                       } else if (oldmem_data.start && from < oldmem_data.size) {
-                               len = min(count, oldmem_data.size - from);
-                               from += oldmem_data.start;
+                       if (oldmem_data.start && src - oldmem_data.start < oldmem_data.size) {
+                               src -= oldmem_data.start;
+                               len = min(count, oldmem_data.size - src);
+                       } else if (oldmem_data.start && src < oldmem_data.size) {
+                               len = min(count, oldmem_data.size - src);
+                               src += oldmem_data.start;
                        } else {
                                len = count;
                        }
@@ -163,7 +162,7 @@ int copy_oldmem_kernel(void *dst, void *src, size_t count)
                        } else {
                                ra = dst;
                        }
-                       if (memcpy_real(ra, (void *) from, len))
+                       if (memcpy_real(ra, src, len))
                                return -EFAULT;
                }
                dst += len;
@@ -176,31 +175,30 @@ int copy_oldmem_kernel(void *dst, void *src, size_t count)
 /*
  * Copy memory of the old, dumped system to a user space virtual address
  */
-static int copy_oldmem_user(void __user *dst, void *src, size_t count)
+static int copy_oldmem_user(void __user *dst, unsigned long src, size_t count)
 {
-       unsigned long from, len;
+       unsigned long len;
        int rc;
 
        while (count) {
-               from = __pa(src);
-               if (!oldmem_data.start && from < sclp.hsa_size) {
+               if (!oldmem_data.start && src < sclp.hsa_size) {
                        /* Copy from zfcp/nvme dump HSA area */
-                       len = min(count, sclp.hsa_size - from);
-                       rc = memcpy_hsa_user(dst, from, len);
+                       len = min(count, sclp.hsa_size - src);
+                       rc = memcpy_hsa_user(dst, src, len);
                        if (rc)
                                return rc;
                } else {
                        /* Check for swapped kdump oldmem areas */
-                       if (oldmem_data.start && from - oldmem_data.start < oldmem_data.size) {
-                               from -= oldmem_data.start;
-                               len = min(count, oldmem_data.size - from);
-                       } else if (oldmem_data.start && from < oldmem_data.size) {
-                               len = min(count, oldmem_data.size - from);
-                               from += oldmem_data.start;
+                       if (oldmem_data.start && src - oldmem_data.start < oldmem_data.size) {
+                               src -= oldmem_data.start;
+                               len = min(count, oldmem_data.size - src);
+                       } else if (oldmem_data.start && src < oldmem_data.size) {
+                               len = min(count, oldmem_data.size - src);
+                               src += oldmem_data.start;
                        } else {
                                len = count;
                        }
-                       rc = copy_to_user_real(dst, (void *) from, count);
+                       rc = copy_to_user_real(dst, src, count);
                        if (rc)
                                return rc;
                }
@@ -217,12 +215,12 @@ static int copy_oldmem_user(void __user *dst, void *src, size_t count)
 ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize,
                         unsigned long offset, int userbuf)
 {
-       void *src;
+       unsigned long src;
        int rc;
 
        if (!csize)
                return 0;
-       src = (void *) (pfn << PAGE_SHIFT) + offset;
+       src = pfn_to_phys(pfn) + offset;
        if (userbuf)
                rc = copy_oldmem_user((void __force __user *) buf, src, csize);
        else
@@ -429,10 +427,10 @@ static void *nt_prpsinfo(void *ptr)
 static void *get_vmcoreinfo_old(unsigned long *size)
 {
        char nt_name[11], *vmcoreinfo;
+       unsigned long addr;
        Elf64_Nhdr note;
-       void *addr;
 
-       if (copy_oldmem_kernel(&addr, (void *)__LC_VMCORE_INFO, sizeof(addr)))
+       if (copy_oldmem_kernel(&addr, __LC_VMCORE_INFO, sizeof(addr)))
                return NULL;
        memset(nt_name, 0, sizeof(nt_name));
        if (copy_oldmem_kernel(&note, addr, sizeof(note)))
index a2b08d3..6b5b64e 100644 (file)
@@ -91,7 +91,7 @@ static void os_info_old_alloc(int nr, int align)
                goto fail;
        }
        buf_align = PTR_ALIGN(buf, align);
-       if (copy_oldmem_kernel(buf_align, (void *) addr, size)) {
+       if (copy_oldmem_kernel(buf_align, addr, size)) {
                msg = "copy failed";
                goto fail_free;
        }
@@ -124,15 +124,14 @@ static void os_info_old_init(void)
                return;
        if (!oldmem_data.start)
                goto fail;
-       if (copy_oldmem_kernel(&addr, (void *)__LC_OS_INFO, sizeof(addr)))
+       if (copy_oldmem_kernel(&addr, __LC_OS_INFO, sizeof(addr)))
                goto fail;
        if (addr == 0 || addr % PAGE_SIZE)
                goto fail;
        os_info_old = kzalloc(sizeof(*os_info_old), GFP_KERNEL);
        if (!os_info_old)
                goto fail;
-       if (copy_oldmem_kernel(os_info_old, (void *) addr,
-                              sizeof(*os_info_old)))
+       if (copy_oldmem_kernel(os_info_old, addr, sizeof(*os_info_old)))
                goto fail_free;
        if (os_info_old->magic != OS_INFO_MAGIC)
                goto fail_free;
index b214af5..4f0e9f4 100644 (file)
@@ -670,7 +670,7 @@ static __init void smp_save_cpu_regs(struct save_area *sa, u16 addr,
                                     bool is_boot_cpu, void *regs)
 {
        if (is_boot_cpu)
-               copy_oldmem_kernel(regs, (void *) __LC_FPREGS_SAVE_AREA, 512);
+               copy_oldmem_kernel(regs, __LC_FPREGS_SAVE_AREA, 512);
        else
                __pcpu_sigp_relax(addr, SIGP_STORE_STATUS_AT_ADDRESS, __pa(regs));
        save_area_add_regs(sa, regs);
index d4d311c..4cc5020 100644 (file)
@@ -121,7 +121,7 @@ static unsigned long __no_sanitize_address _memcpy_real(unsigned long dest,
 /*
  * Copy memory in real mode (kernel to kernel)
  */
-int memcpy_real(void *dest, void *src, size_t count)
+int memcpy_real(void *dest, unsigned long src, size_t count)
 {
        unsigned long _dest  = (unsigned long)dest;
        unsigned long _src   = (unsigned long)src;
@@ -173,7 +173,7 @@ void memcpy_absolute(void *dest, void *src, size_t count)
 /*
  * Copy memory from kernel (real) to user (virtual)
  */
-int copy_to_user_real(void __user *dest, void *src, unsigned long count)
+int copy_to_user_real(void __user *dest, unsigned long src, unsigned long count)
 {
        int offs = 0, size, rc;
        char *buf;
index 3ba2d93..516783b 100644 (file)
@@ -229,8 +229,7 @@ static int __init zcore_reipl_init(void)
                rc = memcpy_hsa_kernel(zcore_ipl_block, ipib_info.ipib,
                                       PAGE_SIZE);
        else
-               rc = memcpy_real(zcore_ipl_block, (void *) ipib_info.ipib,
-                                PAGE_SIZE);
+               rc = memcpy_real(zcore_ipl_block, ipib_info.ipib, PAGE_SIZE);
        if (rc || (__force u32)csum_partial(zcore_ipl_block, zcore_ipl_block->hdr.len, 0) !=
            ipib_info.checksum) {
                TRACE("Checksum does not match\n");