linux-kernel - Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a7znsubp.fsf@on-the-bus.cambridge.arm.com>
Date:   Wed, 15 Nov 2017 10:35:38 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     "Liuwenliang \(Abbott Liu\)" <liuwenliang@...wei.com>
Cc:     "linux\@armlinux.org.uk" <linux@...linux.org.uk>,
        "aryabinin\@virtuozzo.com" <aryabinin@...tuozzo.com>,
        "afzal.mohd.ma\@gmail.com" <afzal.mohd.ma@...il.com>,
        "f.fainelli\@gmail.com" <f.fainelli@...il.com>,
        "labbott\@redhat.com" <labbott@...hat.com>,
        "kirill.shutemov\@linux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "mhocko\@suse.com" <mhocko@...e.com>,
        "cdall\@linaro.org" <cdall@...aro.org>,
        "catalin.marinas\@arm.com" <catalin.marinas@....com>,
        "akpm\@linux-foundation.org" <akpm@...ux-foundation.org>,
        "mawilcox\@microsoft.com" <mawilcox@...rosoft.com>,
        "tglx\@linutronix.de" <tglx@...utronix.de>,
        "thgarnie\@google.com" <thgarnie@...gle.com>,
        "keescook\@chromium.org" <keescook@...omium.org>,
        "arnd\@arndb.de" <arnd@...db.de>,
        "vladimir.murzin\@arm.com" <vladimir.murzin@....com>,
        "tixy\@linaro.org" <tixy@...aro.org>,
        "ard.biesheuvel\@linaro.org" <ard.biesheuvel@...aro.org>,
        "robin.murphy\@arm.com" <robin.murphy@....com>,
        "mingo\@kernel.org" <mingo@...nel.org>,
        "grygorii.strashko\@linaro.org" <grygorii.strashko@...aro.org>,
        "glider\@google.com" <glider@...gle.com>,
        "dvyukov\@google.com" <dvyukov@...gle.com>,
        "opendmb\@gmail.com" <opendmb@...il.com>,
        "linux-arm-kernel\@lists.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        "kasan-dev\@googlegroups.com" <kasan-dev@...glegroups.com>,
        "linux-mm\@kvack.org" <linux-mm@...ck.org>,
        Jiazhenghua <jiazhenghua@...wei.com>,
        Dailei <dylix.dailei@...wei.com>,
        Zengweilin <zengweilin@...wei.com>,
        Heshaoliang <heshaoliang@...wei.com>
Subject: Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" <liuwenliang@...wei.com> wrote:
> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyngier@....com] wrote:
>>On 09/11/17 07:46, Liuwenliang (Abbott Liu) wrote:
>>> diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c
>>> index 049ee0a..359a782 100644
>>> --- a/arch/arm/mm/kasan_init.c
>>> +++ b/arch/arm/mm/kasan_init.c
>>> @@ -15,6 +15,7 @@
>>>  #include <asm/proc-fns.h>
>>>  #include <asm/tlbflush.h>
>>>  #include <asm/cp15.h>
>>> +#include <asm/kvm_hyp.h>
>>
>>No, please don't do that. You shouldn't have to include KVM stuff in
>>unrelated code. Instead of adding stuff to kvm_hyp.h, move all the
>>__ACCESS_CP15* to cp15.h, and it will be obvious to everyone that this
>>is where new definition should be added.
>
> Thanks for your review.  You are right. It is better to move
> __ACCESS_CP15* to cp15.h than to include kvm_hyp.h. But I don't think
> it is a good idea to move registers definition which is used in
> virtualization to cp15.h, Because there is no virtualization stuff in
> cp15.h.

It is not about virtualization at all.

It is about what is a CP15 register and what is not. This file is called
"cp15.h", not "cp15-except-virtualization-and-maybe-some-others.h". But
at the end of the day, that's for Russell to decide.

>
> Here is the code which I tested on vexpress_a15 and vexpress_a9:
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index dbdbce1..6db1f51 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -64,6 +64,43 @@
>  #define __write_sysreg(v, r, w, c, t)  asm volatile(w " " c : : "r" ((t)(v)))
>  #define write_sysreg(v, ...)           __write_sysreg(v, __VA_ARGS__)
>
> +#ifdef CONFIG_ARM_LPAE
> +#define TTBR0           __ACCESS_CP15_64(0, c2)
> +#define TTBR1           __ACCESS_CP15_64(1, c2)
> +#define PAR             __ACCESS_CP15_64(0, c7)
> +#else
> +#define TTBR0           __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1           __ACCESS_CP15(c2, 0, c0, 1)
> +#define PAR             __ACCESS_CP15(c7, 0, c4, 0)
> +#endif

Again: there is no point in not having these register encodings
cohabiting. They are both perfectly defined in the architecture. Just
suffix one (or even both) with their respective size, making it obvious
which one you're talking about.

Thanks,

	M.
-- 
Jazz is not dead, it just smell funny.