linux-kernel - Re: [PATCH 5.7 233/244] RISC-V: Acquire mmap lock before invoking walk_page

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: <194a70d4428b96b59594efc5cae7ed26f6da45b3.camel@wdc.com>
Date:   Mon, 20 Jul 2020 18:50:10 +0000
From:   Atish Patra <Atish.Patra@....com>
To:     "naresh.kamboju@...aro.org" <naresh.kamboju@...aro.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
CC:     "walken@...gle.com" <walken@...gle.com>,
        "palmerdabbelt@...gle.com" <palmerdabbelt@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "zong.li@...ive.com" <zong.li@...ive.com>,
        "lkft-triage@...ts.linaro.org" <lkft-triage@...ts.linaro.org>
Subject: Re: [PATCH 5.7 233/244] RISC-V: Acquire mmap lock before invoking
 walk_page_range

On Mon, 2020-07-20 at 23:11 +0530, Naresh Kamboju wrote:
> RISC-V build breaks on stable-rc 5.7 branch.
> build failed with gcc-8, gcc-9 and gcc-9.
> 

Sorry for the compilation issue.

mmap_read_lock was intrdouced in the following commit.

commit 9740ca4e95b4
Author: Michel Lespinasse <walken@...gle.com>
Date:   Mon Jun 8 21:33:14 2020 -0700

    mmap locking API: initial implementation as rwsem wrappers

The following two commits replaced the usage of mmap_sem rwsem calls
with mmap_lock.

d8ed45c5dcd4 (mmap locking API: use coccinelle to convert mmap_sem
rwsem call sites)
89154dd5313f (mmap locking API: convert mmap_sem call sites missed by
coccinelle)

The first commit is not present in stale 5.7-y for obvious reasons.

Do we need to send a separate patch only for stable branch with
mmap_sem ? I am not sure if that will cause a conflict again in future.

> On Mon, 20 Jul 2020 at 21:46, Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> > From: Atish Patra <atish.patra@....com>
> > 
> > commit 0e2c09011d4de4161f615ff860a605a9186cf62a upstream.
> > 
> > As per walk_page_range documentation, mmap lock should be acquired
> > by the
> > caller before invoking walk_page_range. mmap_assert_locked gets
> > triggered
> > without that. The details can be found here.
> > 
> > http://lists.infradead.org/pipermail/linux-riscv/2020-June/010335.html
> > 
> > Fixes: 395a21ff859c(riscv: add ARCH_HAS_SET_DIRECT_MAP support)
> > Signed-off-by: Atish Patra <atish.patra@....com>
> > Reviewed-by: Michel Lespinasse <walken@...gle.com>
> > Reviewed-by: Zong Li <zong.li@...ive.com>
> > Signed-off-by: Palmer Dabbelt <palmerdabbelt@...gle.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > 
> > ---
> >  arch/riscv/mm/pageattr.c |   14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > --- a/arch/riscv/mm/pageattr.c
> > +++ b/arch/riscv/mm/pageattr.c
> > @@ -151,6 +151,7 @@ int set_memory_nx(unsigned long addr, in
> > 
> >  int set_direct_map_invalid_noflush(struct page *page)
> >  {
> > +       int ret;
> >         unsigned long start = (unsigned long)page_address(page);
> >         unsigned long end = start + PAGE_SIZE;
> >         struct pageattr_masks masks = {
> > @@ -158,11 +159,16 @@ int set_direct_map_invalid_noflush(struc
> >                 .clear_mask = __pgprot(_PAGE_PRESENT)
> >         };
> > 
> > -       return walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > +       mmap_read_lock(&init_mm);
> > +       ret = walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > +       mmap_read_unlock(&init_mm);
> 
> make -sk KBUILD_BUILD_USER=TuxBuild -C/linux ARCH=riscv
> CROSS_COMPILE=riscv64-linux-gnu- HOSTCC=gcc CC="sccache
> riscv64-linux-gnu-gcc" O=build defconfig
> #
> #
> make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j32 ARCH=riscv
> CROSS_COMPILE=riscv64-linux-gnu- HOSTCC=gcc CC="sccache
> riscv64-linux-gnu-gcc" O=build
> #
> ../arch/riscv/mm/pageattr.c: In function
> ‘set_direct_map_invalid_noflush’:
> ../arch/riscv/mm/pageattr.c:162:2: error: implicit declaration of
> function ‘mmap_read_lock’; did you mean ‘_raw_read_lock’?
> [-Werror=implicit-function-declaration]
>   162 |  mmap_read_lock(&init_mm);
>       |  ^~~~~~~~~~~~~~
>       |  _raw_read_lock
> ../arch/riscv/mm/pageattr.c:164:2: error: implicit declaration of
> function ‘mmap_read_unlock’; did you mean ‘_raw_read_unlock’?
> [-Werror=implicit-function-declaration]
>   164 |  mmap_read_unlock(&init_mm);
>       |  ^~~~~~~~~~~~~~~~
>       |  _raw_read_unlock
> cc1: some warnings being treated as errors
> 
> > +
> > +       return ret;
> >  }
> > 
> >  int set_direct_map_default_noflush(struct page *page)
> >  {
> > +       int ret;
> >         unsigned long start = (unsigned long)page_address(page);
> >         unsigned long end = start + PAGE_SIZE;
> >         struct pageattr_masks masks = {
> > @@ -170,7 +176,11 @@ int set_direct_map_default_noflush(struc
> >                 .clear_mask = __pgprot(0)
> >         };
> > 
> > -       return walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > +       mmap_read_lock(&init_mm);
> > +       ret = walk_page_range(&init_mm, start, end, &pageattr_ops,
> > &masks);
> > +       mmap_read_unlock(&init_mm);
> > +
> > +       return ret;
> >  }
> > 
> >  void __kernel_map_pages(struct page *page, int numpages, int
> > enable)
> > 
> 
> ref:
> full build log with default config.
> https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/647154950
> 

-- 
Regards,
Atish