Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200217083223.2011-8-zong.li@sifive.com> <mhng-0d866567-f710-4d27-8ae9-1f78454d7d85@palmerdabbelt-glaptop1>
In-Reply-To: <mhng-0d866567-f710-4d27-8ae9-1f78454d7d85@palmerdabbelt-glaptop1>
From:   Zong Li <zongbox@gmail.com>
Date:   Thu, 5 Mar 2020 13:53:41 +0800
Message-ID: <CA+ZOyahuV=Y427S_dWjOvbfQQNSWS3aK8SXxeEEiGBxt3n8WiA@mail.gmail.com>
Subject: Re: [PATCH 7/8] riscv: add DEBUG_WX support
To:     Palmer Dabbelt <palmer@dabbelt.com>
Cc:     Zong Li <zong.li@sifive.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Paul Walmsley <paul.walmsley@sifive.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Palmer Dabbelt <palmer@dabbelt.com> =E6=96=BC 2020=E5=B9=B43=E6=9C=885=E6=
=97=A5 =E9=80=B1=E5=9B=9B =E4=B8=8A=E5=8D=889:44=E5=AF=AB=E9=81=93=EF=BC=9A
>
> On Mon, 17 Feb 2020 00:32:22 PST (-0800), zong.li@sifive.com wrote:
> > Support DEBUG_WX to check whether there are mapping with write and
> > execute permission at the same time.
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> >  arch/riscv/Kconfig.debug        | 30 ++++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/ptdump.h |  6 ++++++
> >  arch/riscv/mm/init.c            |  2 ++
> >  3 files changed, 38 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig.debug b/arch/riscv/Kconfig.debug
> > index e69de29bb2d1..2bcd88e75626 100644
> > --- a/arch/riscv/Kconfig.debug
> > +++ b/arch/riscv/Kconfig.debug
> > @@ -0,0 +1,30 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +config DEBUG_WX
> > +     bool "Warn on W+X mappings at boot"
> > +     select PTDUMP_CORE
> > +     help
> > +       Generate a warning if any W+X mappings are found at boot.
> > +
> > +       This is useful for discovering cases where the kernel is leavin=
g
> > +       W+X mappings after applying NX, as such mappings are a security=
 risk.
> > +       This check also includes UXN, which should be set on all kernel
> > +       mappings.
> > +
> > +       Look for a message in dmesg output like this:
> > +
> > +         riscv/mm: Checked W+X mappings: passed, no W+X pages found.
> > +
> > +       or like this, if the check failed:
> > +
> > +         riscv/mm: Checked W+X mappings: FAILED, <N> W+X pages found.
> > +
> > +       Note that even if the check fails, your kernel is possibly
> > +       still fine, as W+X mappings are not a security hole in
> > +       themselves, what they do is that they make the exploitation
> > +       of other unfixed kernel bugs easier.
> > +
> > +       There is no runtime or memory usage effect of this option
> > +       once the kernel has booted up - it's a one time check.
> > +
> > +       If in doubt, say "Y".
>
> It looks like this comes verbatim from the arm64 port, at least.  I think=
 we
> should just refactor this to some sort of ARCH_HAS_DEBUG_WX so we can sha=
re the
> code.  I usually do this by adding the shared support, using it for RISC-=
V, and
> then converting the other ports over.
>

OK. It seems to be different work, maybe I could separate this patch
in next version. Thanks.

> > diff --git a/arch/riscv/include/asm/ptdump.h b/arch/riscv/include/asm/p=
tdump.h
> > index e29af7191909..eb2a1cc5f22c 100644
> > --- a/arch/riscv/include/asm/ptdump.h
> > +++ b/arch/riscv/include/asm/ptdump.h
> > @@ -8,4 +8,10 @@
> >
> >  void ptdump_check_wx(void);
> >
> > +#ifdef CONFIG_DEBUG_WX
> > +#define debug_checkwx() ptdump_check_wx()
> > +#else
> > +#define debug_checkwx() do { } while (0)
> > +#endif
> > +
> >  #endif /* _ASM_RISCV_PTDUMP_H */
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 09fa643e079c..a05d76e5fefe 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -509,6 +509,8 @@ void mark_rodata_ro(void)
> >       set_memory_ro(rodata_start, (data_start - rodata_start) >> PAGE_S=
HIFT);
> >       set_memory_nx(rodata_start, (data_start - rodata_start) >> PAGE_S=
HIFT);
> >       set_memory_nx(data_start, (max_low - data_start) >> PAGE_SHIFT);
> > +
> > +     debug_checkwx();
> >  }
> >  #endif
>