[PATCH 1/3] powerpc/va: Add a __va() variant that doesn't do input validation

[PATCH 1/3] powerpc/va: Add a __va() variant that doesn't do input validation

[Aneesh Kumar K.V]

On ppc64, __va(x) do check for input argument to be less than PAGE_OFFSET.
In certain code paths, we want to skip that check. Add a variant ___va(x)
to be used in such cases.

Switch the #define to static inline. __pa() still doesn't benefit from this. But
a static inline done in this patch is better than multi-line #define.
For __va() we get the type checking benefit. We still have to keep the
macro __pa(x) to avoid a large number of compilation errors with the change.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/include/asm/page.h         | 38 ++++++++++++++++---------
 arch/powerpc/mm/nohash/book3e_pgtable.c |  2 +-
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 3ee8df0f66e0..a3a2725a80ab 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -9,6 +9,7 @@
 #ifndef __ASSEMBLY__
 #include <linux/types.h>
 #include <linux/kernel.h>
+#include <linux/mmdebug.h>
 #else
 #include <asm/types.h>
 #endif
@@ -208,30 +209,41 @@ static inline bool pfn_valid(unsigned long pfn)
  * the other definitions for __va & __pa.
  */
 #if defined(CONFIG_PPC32) && defined(CONFIG_BOOKE)
-#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + VIRT_PHYS_OFFSET))
+#define ___va(x) ((void *)(unsigned long)((phys_addr_t)(x) + VIRT_PHYS_OFFSET))
 #define __pa(x) ((phys_addr_t)(unsigned long)(x) - VIRT_PHYS_OFFSET)
+#define __va(x) ___va(x)
 #else
 #ifdef CONFIG_PPC64
+
+#ifndef __ASSEMBLY__
 /*
  * gcc miscompiles (unsigned long)(&static_var) - PAGE_OFFSET
  * with -mcmodel=medium, so we use & and | instead of - and + on 64-bit.
  * This also results in better code generation.
  */
-#define __va(x)								\
-({									\
-	VIRTUAL_BUG_ON((unsigned long)(x) >= PAGE_OFFSET);		\
-	(void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET);	\
-})
-
-#define __pa(x)								\
-({									\
-	VIRTUAL_BUG_ON((unsigned long)(x) < PAGE_OFFSET);		\
-	(unsigned long)(x) & 0x0fffffffffffffffUL;			\
-})
+static inline void *___va(phys_addr_t addr)
+{
+	return (void *)(addr | PAGE_OFFSET);
+}
+
+static inline void *__va(phys_addr_t addr)
+{
+	VIRTUAL_BUG_ON((unsigned long)(addr) >= PAGE_OFFSET);
+	return ___va(addr);
+}
+
+static inline phys_addr_t ___pa(void *addr)
+{
+	VIRTUAL_BUG_ON((unsigned long)(addr) < PAGE_OFFSET);
+	return (phys_addr_t)((unsigned long)addr & 0x0fffffffffffffffUL);
+}
+#define __pa(x) ___pa((void *)(x))
+#endif /*  __ASSEMBLY__ */
 
 #else /* 32-bit, non book E */
-#define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + PAGE_OFFSET - MEMORY_START))
+#define ___va(x) ((void *)(unsigned long)((phys_addr_t)(x) + PAGE_OFFSET - MEMORY_START))
 #define __pa(x) ((unsigned long)(x) - PAGE_OFFSET + MEMORY_START)
+#define __va(x) ___va(x)
 #endif
 #endif
 
diff --git a/arch/powerpc/mm/nohash/book3e_pgtable.c b/arch/powerpc/mm/nohash/book3e_pgtable.c
index 4637fdd469cf..a8ce309ce740 100644
--- a/arch/powerpc/mm/nohash/book3e_pgtable.c
+++ b/arch/powerpc/mm/nohash/book3e_pgtable.c
@@ -60,7 +60,7 @@ static void __init *early_alloc_pgtable(unsigned long size)
 
 	if (!ptr)
 		panic("%s: Failed to allocate %lu bytes align=0x%lx max_addr=%lx\n",
-		      __func__, size, size, __pa(MAX_DMA_ADDRESS));
+		      __func__, size, size, (unsigned long)__pa(MAX_DMA_ADDRESS));
 
 	return ptr;
 }
-- 
2.26.2

[PATCH 2/3] powerpc: Fix instruction dumping to use address value correctly

[Aneesh Kumar K.V]

Avoid updating address value in the loop. If in real mode, convert the address
outside the loop. Also, use ___va() to convert the real address that will
skip the input validation. We can get interrupts with IR=0 and with NIP
value > PAGE_OFFSET.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/include/asm/processor.h |  9 +++++++++
 arch/powerpc/kernel/process.c        | 18 ++++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 591987da44e2..4e7a5adc04df 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -436,6 +436,15 @@ extern void cvt_fd(float *from, double *to);
 extern void cvt_df(double *from, float *to);
 extern void _nmask_and_or_msr(unsigned long nmask, unsigned long or_val);
 
+static inline unsigned long fixup_real_addr(struct pt_regs *regs,
+					    unsigned long addr)
+{
+	if (!(regs->msr & MSR_IR))
+		return (unsigned long)___va(addr);
+
+	return addr;
+}
+
 #ifdef CONFIG_PPC64
 /*
  * We handle most unaligned accesses in hardware. On the other hand 
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 682deeee421f..36cc03c33d25 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1238,29 +1238,31 @@ struct task_struct *__switch_to(struct task_struct *prev,
 static void show_instructions(struct pt_regs *regs)
 {
 	int i;
+	unsigned long nip = regs->nip;
 	unsigned long pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
 
 	printk("Instruction dump:");
 
+
+#if !defined(CONFIG_BOOKE)
+	/* If executing with the IMMU off, adjust pc rather
+	 * than print XXXXXXXX.
+	 */
+	pc = fixup_real_addr(regs, pc);
+	nip = fixup_real_addr(regs, nip);
+#endif
 	for (i = 0; i < NR_INSN_TO_PRINT; i++) {
 		int instr;
 
 		if (!(i % 8))
 			pr_cont("\n");
 
-#if !defined(CONFIG_BOOKE)
-		/* If executing with the IMMU off, adjust pc rather
-		 * than print XXXXXXXX.
-		 */
-		if (!(regs->msr & MSR_IR))
-			pc = (unsigned long)phys_to_virt(pc);
-#endif
 
 		if (!__kernel_text_address(pc) ||
 		    probe_kernel_address((const void *)pc, instr)) {
 			pr_cont("XXXXXXXX ");
 		} else {
-			if (regs->nip == pc)
+			if (nip == pc)
 				pr_cont("<%08x> ", instr);
 			else
 				pr_cont("%08x ", instr);
-- 
2.26.2

[PATCH 3/3] powerp: Avoid opencoding fixup_real_addr

[Aneesh Kumar K.V]

Use the newly added helper.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/kernel/traps.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a47fb49b7af8..503097c6aab2 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1479,12 +1479,10 @@ void program_check_exception(struct pt_regs *regs)
 				== NOTIFY_STOP)
 			goto bail;
 
-		bugaddr = regs->nip;
 		/*
 		 * Fixup bugaddr for BUG_ON() in real mode
 		 */
-		if (!is_kernel_addr(bugaddr) && !(regs->msr & MSR_IR))
-			bugaddr += PAGE_OFFSET;
+		bugaddr = fixup_real_addr(regs, regs->nip);
 
 		if (!(regs->msr & MSR_PR) &&  /* not user-mode */
 		    report_bug(bugaddr, regs) == BUG_TRAP_TYPE_WARN) {
-- 
2.26.2

Re: [PATCH 1/3] powerpc/va: Add a __va() variant that doesn't do input validation

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6677 bytes --]

Hi "Aneesh,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.7-rc4 next-20200507]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Aneesh-Kumar-K-V/powerpc-va-Add-a-__va-variant-that-doesn-t-do-input-validation/20200508-042829
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   arch/powerpc/platforms/powernv/opal-core.c: In function 'read_opalcore':
>> arch/powerpc/platforms/powernv/opal-core.c:199:20: warning: passing argument 1 of '__va' makes integer from pointer without a cast [-Wint-conversion]
     199 |    memcpy(to, __va(addr), tsz);
         |                    ^~~~
         |                    |
         |                    void *
   In file included from arch/powerpc/include/asm/mmu.h:132,
                    from arch/powerpc/include/asm/lppaca.h:47,
                    from arch/powerpc/include/asm/paca.h:17,
                    from arch/powerpc/include/asm/current.h:13,
                    from include/linux/thread_info.h:21,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/powerpc/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:78,
                    from include/linux/spinlock.h:51,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:6,
                    from include/linux/mm.h:10,
                    from include/linux/memblock.h:13,
                    from arch/powerpc/platforms/powernv/opal-core.c:11:
   arch/powerpc/include/asm/page.h:229:38: note: expected 'phys_addr_t' {aka 'long long unsigned int'} but argument is of type 'void *'
     229 | static inline void *__va(phys_addr_t addr)
         |                          ~~~~~~~~~~~~^~~~

vim +/__va +199 arch/powerpc/platforms/powernv/opal-core.c

6f713d18144ce86 Hari Bathini 2019-09-11  156  
6f713d18144ce86 Hari Bathini 2019-09-11  157  /*
6f713d18144ce86 Hari Bathini 2019-09-11  158   * Read from the ELF header and then the crash dump.
6f713d18144ce86 Hari Bathini 2019-09-11  159   * Returns number of bytes read on success, -errno on failure.
6f713d18144ce86 Hari Bathini 2019-09-11  160   */
6f713d18144ce86 Hari Bathini 2019-09-11  161  static ssize_t read_opalcore(struct file *file, struct kobject *kobj,
6f713d18144ce86 Hari Bathini 2019-09-11  162  			     struct bin_attribute *bin_attr, char *to,
6f713d18144ce86 Hari Bathini 2019-09-11  163  			     loff_t pos, size_t count)
6f713d18144ce86 Hari Bathini 2019-09-11  164  {
6f713d18144ce86 Hari Bathini 2019-09-11  165  	struct opalcore *m;
6f713d18144ce86 Hari Bathini 2019-09-11  166  	ssize_t tsz, avail;
6f713d18144ce86 Hari Bathini 2019-09-11  167  	loff_t tpos = pos;
6f713d18144ce86 Hari Bathini 2019-09-11  168  
6f713d18144ce86 Hari Bathini 2019-09-11  169  	if (pos >= oc_conf->opalcore_size)
6f713d18144ce86 Hari Bathini 2019-09-11  170  		return 0;
6f713d18144ce86 Hari Bathini 2019-09-11  171  
6f713d18144ce86 Hari Bathini 2019-09-11  172  	/* Adjust count if it goes beyond opalcore size */
6f713d18144ce86 Hari Bathini 2019-09-11  173  	avail = oc_conf->opalcore_size - pos;
6f713d18144ce86 Hari Bathini 2019-09-11  174  	if (count > avail)
6f713d18144ce86 Hari Bathini 2019-09-11  175  		count = avail;
6f713d18144ce86 Hari Bathini 2019-09-11  176  
6f713d18144ce86 Hari Bathini 2019-09-11  177  	if (count == 0)
6f713d18144ce86 Hari Bathini 2019-09-11  178  		return 0;
6f713d18144ce86 Hari Bathini 2019-09-11  179  
6f713d18144ce86 Hari Bathini 2019-09-11  180  	/* Read ELF core header and/or PT_NOTE segment */
6f713d18144ce86 Hari Bathini 2019-09-11  181  	if (tpos < oc_conf->opalcorebuf_sz) {
6f713d18144ce86 Hari Bathini 2019-09-11  182  		tsz = min_t(size_t, oc_conf->opalcorebuf_sz - tpos, count);
6f713d18144ce86 Hari Bathini 2019-09-11  183  		memcpy(to, oc_conf->opalcorebuf + tpos, tsz);
6f713d18144ce86 Hari Bathini 2019-09-11  184  		to += tsz;
6f713d18144ce86 Hari Bathini 2019-09-11  185  		tpos += tsz;
6f713d18144ce86 Hari Bathini 2019-09-11  186  		count -= tsz;
6f713d18144ce86 Hari Bathini 2019-09-11  187  	}
6f713d18144ce86 Hari Bathini 2019-09-11  188  
6f713d18144ce86 Hari Bathini 2019-09-11  189  	list_for_each_entry(m, &opalcore_list, list) {
6f713d18144ce86 Hari Bathini 2019-09-11  190  		/* nothing more to read here */
6f713d18144ce86 Hari Bathini 2019-09-11  191  		if (count == 0)
6f713d18144ce86 Hari Bathini 2019-09-11  192  			break;
6f713d18144ce86 Hari Bathini 2019-09-11  193  
6f713d18144ce86 Hari Bathini 2019-09-11  194  		if (tpos < m->offset + m->size) {
6f713d18144ce86 Hari Bathini 2019-09-11  195  			void *addr;
6f713d18144ce86 Hari Bathini 2019-09-11  196  
6f713d18144ce86 Hari Bathini 2019-09-11  197  			tsz = min_t(size_t, m->offset + m->size - tpos, count);
6f713d18144ce86 Hari Bathini 2019-09-11  198  			addr = (void *)(m->paddr + tpos - m->offset);
6f713d18144ce86 Hari Bathini 2019-09-11 @199  			memcpy(to, __va(addr), tsz);
6f713d18144ce86 Hari Bathini 2019-09-11  200  			to += tsz;
6f713d18144ce86 Hari Bathini 2019-09-11  201  			tpos += tsz;
6f713d18144ce86 Hari Bathini 2019-09-11  202  			count -= tsz;
6f713d18144ce86 Hari Bathini 2019-09-11  203  		}
6f713d18144ce86 Hari Bathini 2019-09-11  204  	}
6f713d18144ce86 Hari Bathini 2019-09-11  205  
6f713d18144ce86 Hari Bathini 2019-09-11  206  	return (tpos - pos);
6f713d18144ce86 Hari Bathini 2019-09-11  207  }
6f713d18144ce86 Hari Bathini 2019-09-11  208  

:::::: The code at line 199 was first introduced by commit
:::::: 6f713d18144ce86c9f01cdf64222d6339e26129e powerpc/opalcore: export /sys/firmware/opal/core for analysing opal crashes

:::::: TO: Hari Bathini <hbathini@linux.vnet.ibm.com>
:::::: CC: Michael Ellerman <mpe@ellerman.id.au>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66039 bytes --]