From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753173AbdJSMwG (ORCPT ); Thu, 19 Oct 2017 08:52:06 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:60812 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752869AbdJSMwF (ORCPT ); Thu, 19 Oct 2017 08:52:05 -0400 Date: Thu, 19 Oct 2017 13:51:33 +0100 From: Russell King - ARM Linux To: "Liuwenliang (Lamb)" Cc: Dmitry Vyukov , Andrew Morton , Andrey Ryabinin , "afzal.mohd.ma@gmail.com" , "f.fainelli@gmail.com" , Laura Abbott , "Kirill A. Shutemov" , Michal Hocko , "cdall@linaro.org" , "marc.zyngier@arm.com" , Catalin Marinas , Matthew Wilcox , Thomas Gleixner , Thomas Garnier , Kees Cook , Arnd Bergmann , Vladimir Murzin , "tixy@linaro.org" , Ard Biesheuvel , "robin.murphy@arm.com" , Ingo Molnar , "grygorii.strashko@linaro.org" , Alexander Potapenko , "opendmb@gmail.com" , "linux-arm-kernel@lists.infradead.org" , LKML , kasan-dev , "linux-mm@kvack.org" , Jiazhenghua , Dailei , Zengweilin , Heshaoliang Subject: Re: [PATCH 06/11] change memory_is_poisoned_16 for aligned error Message-ID: <20171019125133.GA20805@n2100.armlinux.org.uk> References: <20171011082227.20546-1-liuwenliang@huawei.com> <20171011082227.20546-7-liuwenliang@huawei.com> <20171011162345.f601c29d12c81af85bf38565@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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() ? -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up