Re: [PATCH] arm: fix page faults in do_alignment

[kbuild test robot <lkp@intel.com>]

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

Hi Jing,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on arm/for-next]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jing-Xiangfeng/arm-fix-page-faults-in-do_alignment/20190831-173417
base:   git://git.armlinux.org.uk/~rmk/linux-arm.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.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
        GCC_VERSION=7.4.0 make.cross ARCH=arm 

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

All warnings (new ones prefixed by >>):

   arch/arm/mm/alignment.c: In function 'do_alignment':
>> arch/arm/mm/alignment.c:792:28: warning: passing argument 1 of '__copy_from_user' makes pointer from integer without a cast [-Wint-conversion]
      fault = __copy_from_user(tinstr,
                               ^~~~~~
   In file included from include/linux/sched/task.h:11:0,
                    from include/linux/sched/signal.h:9,
                    from arch/arm/mm/alignment.c:20:
   include/linux/uaccess.h:67:1: note: expected 'void *' but argument is of type 'u16 {aka short unsigned int}'
    __copy_from_user(void *to, const void __user *from, unsigned long n)
    ^~~~~~~~~~~~~~~~
   arch/arm/mm/alignment.c:801:30: warning: passing argument 1 of '__copy_from_user' makes pointer from integer without a cast [-Wint-conversion]
        fault = __copy_from_user(tinst2,
                                 ^~~~~~
   In file included from include/linux/sched/task.h:11:0,
                    from include/linux/sched/signal.h:9,
                    from arch/arm/mm/alignment.c:20:
   include/linux/uaccess.h:67:1: note: expected 'void *' but argument is of type 'u16 {aka short unsigned int}'
    __copy_from_user(void *to, const void __user *from, unsigned long n)
    ^~~~~~~~~~~~~~~~
   arch/arm/mm/alignment.c:813:28: warning: passing argument 1 of '__copy_from_user' makes pointer from integer without a cast [-Wint-conversion]
      fault = __copy_from_user(instr,
                               ^~~~~
   In file included from include/linux/sched/task.h:11:0,
                    from include/linux/sched/signal.h:9,
                    from arch/arm/mm/alignment.c:20:
   include/linux/uaccess.h:67:1: note: expected 'void *' but argument is of type 'long unsigned int'
    __copy_from_user(void *to, const void __user *from, unsigned long n)
    ^~~~~~~~~~~~~~~~

vim +/__copy_from_user +792 arch/arm/mm/alignment.c

   769	
   770	static int
   771	do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
   772	{
   773		union offset_union uninitialized_var(offset);
   774		unsigned long instr = 0, instrptr;
   775		int (*handler)(unsigned long addr, unsigned long instr, struct pt_regs *regs);
   776		unsigned int type;
   777		mm_segment_t fs;
   778		unsigned int fault;
   779		u16 tinstr = 0;
   780		int isize = 4;
   781		int thumb2_32b = 0;
   782	
   783		if (interrupts_enabled(regs))
   784			local_irq_enable();
   785	
   786		instrptr = instruction_pointer(regs);
   787	
   788		fs = get_fs();
   789		set_fs(KERNEL_DS);
   790		if (thumb_mode(regs)) {
   791			u16 *ptr = (u16 *)(instrptr & ~1);
 > 792			fault = __copy_from_user(tinstr,
   793					(__force const void __user *)ptr,
   794					sizeof(tinstr));
   795			tinstr = __mem_to_opcode_thumb16(tinstr);
   796			if (!fault) {
   797				if (cpu_architecture() >= CPU_ARCH_ARMv7 &&
   798				    IS_T32(tinstr)) {
   799					/* Thumb-2 32-bit */
   800					u16 tinst2 = 0;
   801					fault = __copy_from_user(tinst2,
   802							(__force const void __user *)(ptr+1),
   803							sizeof(tinst2));
   804					tinst2 = __mem_to_opcode_thumb16(tinst2);
   805					instr = __opcode_thumb32_compose(tinstr, tinst2);
   806					thumb2_32b = 1;
   807				} else {
   808					isize = 2;
   809					instr = thumb2arm(tinstr);
   810				}
   811			}
   812		} else {
   813			fault = __copy_from_user(instr,
   814					(__force const void __user *)instrptr,
   815					sizeof(instr));
   816			instr = __mem_to_opcode_arm(instr);
   817		}
   818	
   819		set_fs(fs);
   820		if (fault) {
   821			type = TYPE_FAULT;
   822			goto bad_or_fault;
   823		}
   824	
   825		if (user_mode(regs))
   826			goto user;
   827	
   828		ai_sys += 1;
   829		ai_sys_last_pc = (void *)instruction_pointer(regs);
   830	
   831	 fixup:
   832	
   833		regs->ARM_pc += isize;
   834	
   835		switch (CODING_BITS(instr)) {
   836		case 0x00000000:	/* 3.13.4 load/store instruction extensions */
   837			if (LDSTHD_I_BIT(instr))
   838				offset.un = (instr & 0xf00) >> 4 | (instr & 15);
   839			else
   840				offset.un = regs->uregs[RM_BITS(instr)];
   841	
   842			if ((instr & 0x000000f0) == 0x000000b0 || /* LDRH, STRH */
   843			    (instr & 0x001000f0) == 0x001000f0)   /* LDRSH */
   844				handler = do_alignment_ldrhstrh;
   845			else if ((instr & 0x001000f0) == 0x000000d0 || /* LDRD */
   846				 (instr & 0x001000f0) == 0x000000f0)   /* STRD */
   847				handler = do_alignment_ldrdstrd;
   848			else if ((instr & 0x01f00ff0) == 0x01000090) /* SWP */
   849				goto swp;
   850			else
   851				goto bad;
   852			break;
   853	
   854		case 0x04000000:	/* ldr or str immediate */
   855			if (COND_BITS(instr) == 0xf0000000) /* NEON VLDn, VSTn */
   856				goto bad;
   857			offset.un = OFFSET_BITS(instr);
   858			handler = do_alignment_ldrstr;
   859			break;
   860	
   861		case 0x06000000:	/* ldr or str register */
   862			offset.un = regs->uregs[RM_BITS(instr)];
   863	
   864			if (IS_SHIFT(instr)) {
   865				unsigned int shiftval = SHIFT_BITS(instr);
   866	
   867				switch(SHIFT_TYPE(instr)) {
   868				case SHIFT_LSL:
   869					offset.un <<= shiftval;
   870					break;
   871	
   872				case SHIFT_LSR:
   873					offset.un >>= shiftval;
   874					break;
   875	
   876				case SHIFT_ASR:
   877					offset.sn >>= shiftval;
   878					break;
   879	
   880				case SHIFT_RORRRX:
   881					if (shiftval == 0) {
   882						offset.un >>= 1;
   883						if (regs->ARM_cpsr & PSR_C_BIT)
   884							offset.un |= 1 << 31;
   885					} else
   886						offset.un = offset.un >> shiftval |
   887								  offset.un << (32 - shiftval);
   888					break;
   889				}
   890			}
   891			handler = do_alignment_ldrstr;
   892			break;
   893	
   894		case 0x08000000:	/* ldm or stm, or thumb-2 32bit instruction */
   895			if (thumb2_32b) {
   896				offset.un = 0;
   897				handler = do_alignment_t32_to_handler(&instr, regs, &offset);
   898			} else {
   899				offset.un = 0;
   900				handler = do_alignment_ldmstm;
   901			}
   902			break;
   903	
   904		default:
   905			goto bad;
   906		}
   907	
   908		if (!handler)
   909			goto bad;
   910		type = handler(addr, instr, regs);
   911	
   912		if (type == TYPE_ERROR || type == TYPE_FAULT) {
   913			regs->ARM_pc -= isize;
   914			goto bad_or_fault;
   915		}
   916	
   917		if (type == TYPE_LDST)
   918			do_alignment_finish_ldst(addr, instr, regs, offset);
   919	
   920		return 0;
   921	
   922	 bad_or_fault:
   923		if (type == TYPE_ERROR)
   924			goto bad;
   925		/*
   926		 * We got a fault - fix it up, or die.
   927		 */
   928		do_bad_area(addr, fsr, regs);
   929		return 0;
   930	
   931	 swp:
   932		pr_err("Alignment trap: not handling swp instruction\n");
   933	
   934	 bad:
   935		/*
   936		 * Oops, we didn't handle the instruction.
   937		 */
   938		pr_err("Alignment trap: not handling instruction "
   939			"%0*lx at [<%08lx>]\n",
   940			isize << 1,
   941			isize == 2 ? tinstr : instr, instrptr);
   942		ai_skipped += 1;
   943		return 1;
   944	
   945	 user:
   946		ai_user += 1;
   947	
   948		if (ai_usermode & UM_WARN)
   949			printk("Alignment trap: %s (%d) PC=0x%08lx Instr=0x%0*lx "
   950			       "Address=0x%08lx FSR 0x%03x\n", current->comm,
   951				task_pid_nr(current), instrptr,
   952				isize << 1,
   953				isize == 2 ? tinstr : instr,
   954			        addr, fsr);
   955	
   956		if (ai_usermode & UM_FIXUP)
   957			goto fixup;
   958	
   959		if (ai_usermode & UM_SIGNAL) {
   960			force_sig_fault(SIGBUS, BUS_ADRALN, (void __user *)addr);
   961		} else {
   962			/*
   963			 * We're about to disable the alignment trap and return to
   964			 * user space.  But if an interrupt occurs before actually
   965			 * reaching user space, then the IRQ vector entry code will
   966			 * notice that we were still in kernel space and therefore
   967			 * the alignment trap won't be re-enabled in that case as it
   968			 * is presumed to be always on from kernel space.
   969			 * Let's prevent that race by disabling interrupts here (they
   970			 * are disabled on the way back to user space anyway in
   971			 * entry-common.S) and disable the alignment trap only if
   972			 * there is no work pending for this thread.
   973			 */
   974			raw_local_irq_disable();
   975			if (!(current_thread_info()->flags & _TIF_WORK_MASK))
   976				set_cr(cr_no_alignment);
   977		}
   978	
   979		return 0;
   980	}
   981	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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