On Nov 23, 2017 20:30 Russell King - ARM Linux [mailto:linux@xxxxxxxxxxxxxxx] wrote: >On Thu, Oct 12, 2017 at 11:27:40AM +0000, Liuwenliang (Lamb) wrote: >> >> - I don't understand why this is necessary. memory_is_poisoned_16() >> >> already handles unaligned addresses? >> >> >> >> - If it's needed on ARM then presumably it will be needed on other >> >> architectures, so CONFIG_ARM is insufficiently general. >> >> >> >> - If the present memory_is_poisoned_16() indeed doesn't work on ARM, >> >> it would be better to generalize/fix it in some fashion rather than >> >> creating a new variant of the function. >> >> >> >Yes, I think it will be better to fix the current function rather then >> >have 2 slightly different copies with ifdef's. >> >Will something along these lines work for arm? 16-byte accesses are >> >not too common, so it should not be a performance problem. And >> >probably modern compilers can turn 2 1-byte checks into a 2-byte check >> >where safe (x86). >> >> >static __always_inline bool memory_is_poisoned_16(unsigned long addr) >> >{ >> > u8 *shadow_addr = (u8 *)kasan_mem_to_shadow((void *)addr); >> > >> > if (shadow_addr[0] || shadow_addr[1]) >> > return true; >> > /* Unaligned 16-bytes access maps into 3 shadow bytes. */ >> > if (unlikely(!IS_ALIGNED(addr, KASAN_SHADOW_SCALE_SIZE))) >> > return memory_is_poisoned_1(addr + 15); >> > return false; >> >} >> >> Thanks for Andrew Morton and Dmitry Vyukov's review. >> If the parameter addr=0xc0000008, now in function: >> static __always_inline bool memory_is_poisoned_16(unsigned long addr) >> { >> --- //shadow_addr = (u16 *)(KASAN_OFFSET+0x18000001(=0xc0000008>>3)) is not >> --- // unsigned by 2 bytes. >> u16 *shadow_addr = (u16 *)kasan_mem_to_shadow((void *)addr); >> >> /* Unaligned 16-bytes access maps into 3 shadow bytes. */ >> if (unlikely(!IS_ALIGNED(addr, KASAN_SHADOW_SCALE_SIZE))) >> return *shadow_addr || memory_is_poisoned_1(addr + 15); >> ---- //here is going to be error on arm, specially when kernel has not finished yet. >> ---- //Because the unsigned accessing cause DataAbort Exception which is not >> ---- //initialized when kernel is starting. >> return *shadow_addr; >> } >> >> I also think it is better to fix this problem. >What about using get_unaligned() ? Thanks for your review. I think it is good idea to use get_unaligned. But ARMv7 support CONFIG_ HAVE_EFFICIENT_UNALIGNED_ACCESS (arch/arm/Kconfig : select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU). So on ARMv7, the code: u16 *shadow_addr = get_unaligned((u16 *)kasan_mem_to_shadow((void *)addr)); equals the code:000 u16 *shadow_addr = (u16 *)kasan_mem_to_shadow((void *)addr); On ARMv7, if SCRLR.A is 0, unaligned access is OK. Here is the description comes from ARM(r) Architecture Reference Manual ARMv7-A and ARMv7-R edition : A3.2.1 Unaligned data access An ARMv7 implementation must support unaligned data accesses by some load and store instructions, as Table A3-1 shows. Software can set the SCTLR.A bit to control whether a misaligned access by one of these instructions causes an Alignment fault Data abort exception. Table A3-1 Alignment requirements of load/store instructions Instructions Alignment check SCTLR.A is 0 SCTLR.A is 1 LDRB, LDREXB, LDRBT, LDRSB, LDRSBT, STRB, None - - STREXB, STRBT, SWPB, TBB LDRH, LDRHT, LDRSH, LDRSHT, STRH, STRHT, Halfword Unaligned access Alignment fault TBH LDREXH, STREXH Halfword Alignment fault Alignment fault LDR, LDRT, STR, STRT PUSH, encodings T3 and A2 only Word Unaligned access Alignment fault POP, encodings T3 and A2 only LDREX, STREX Word Alignment fault Alignment fault LDREXD, STREXD Doubleword Alignment fault Alignment fault All forms of LDM and STM, LDRD, RFE, SRS, STRD, SWP PUSH, except for encodings T3 and A2 Word Alignment fault Alignment fault POP, except for encodings T3 and A2 LDC, LDC2, STC, STC2 Word Alignment fault Alignment fault VLDM, VLDR, VPOP, VPUSH, VSTM, VSTR Word Alignment fault Alignment fault VLD1, VLD2, VLD3, VLD4, VST1, VST2, VST3, VST4, Element size Unaligned access Alignment fault all with standard alignmenta VLD1, VLD2, VLD3, VLD4, VST1, VST2, VST3, VST4, As specified by@<align> Alignment fault Alignment fault all with @<align> specifieda On ARMv7, the following code can guarantee that if SCRLR.A is 0: __enable_mmu: #if defined(CONFIG_ALIGNMENT_TRAP) && __LINUX_ARM_ARCH__ < 6 orr r0, r0, #CR_A #else bic r0, r0, #CR_A //clear CR_A #endif #ifdef CONFIG_CPU_DCACHE_DISABLE bic r0, r0, #CR_C #endif #ifdef CONFIG_CPU_BPREDICT_DISABLE bic r0, r0, #CR_Z #endif #ifdef CONFIG_CPU_ICACHE_DISABLE bic r0, r0, #CR_I #endif #ifdef CONFIG_ARM_LPAE mcrr p15, 0, r4, r5, c2 @ load TTBR0 #else mov r5, #DACR_INIT mcr p15, 0, r5, c3, c0, 0 @ load domain access register mcr p15, 0, r4, c2, c0, 0 @ load page table pointer #endif b __turn_mmu_on ENDPROC(__enable_mmu) /* * Enable the MMU. This completely changes the structure of the visible * memory space. You will not be able to trace execution through this. * If you have an enquiry about this, *please* check the linux-arm-kernel * mailing list archives BEFORE sending another post to the list. * * r0 = cp#15 control register * r1 = machine ID * r2 = atags or dtb pointer * r9 = processor ID * r13 = *virtual* address to jump to upon completion * * other registers depend on the function called upon completion */ .align 5 .pushsection .idmap.text, "ax" ENTRY(__turn_mmu_on) mov r0, r0 instr_sync mcr p15, 0, r0, c1, c0, 0 @ write control reg //here set SCTLR=r0 mrc p15, 0, r3, c0, c0, 0 @ read id reg instr_sync mov r3, r3 mov r3, r13 ret r3 __turn_mmu_on_end: ENDPROC(__turn_mmu_on) So the following code is OK: static __always_inline bool memory_is_poisoned_16(unsigned long addr) { - u16 *shadow_addr = (u16 *)kasan_mem_to_shadow((void *)addr); + u16 *shadow_addr = get_unaligned( (u16 *)kasan_mem_to_shadow((void *)addr)); /* Unaligned 16-bytes access maps into 3 shadow bytes. */ if (unlikely(!IS_ALIGNED(addr, KASAN_SHADOW_SCALE_SIZE))) return *shadow_addr || memory_is_poisoned_1(addr + 15); return *shadow_addr; } A very good suggestion, Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href
