Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
MIME-Version: 1.0
References: <000000000000773fa7060a31e2cc@google.com> <CANaxB-yrvmv134dwTcMD9q5chXvm3YU1pDFhqvaRA8M1Gn7Guw@mail.gmail.com>
 <ZVVoCT_gNvbZg93f@x1n>
In-Reply-To: <ZVVoCT_gNvbZg93f@x1n>
From:   Andrei Vagin <avagin@gmail.com>
Date:   Thu, 16 Nov 2023 07:38:00 -0800
Message-ID: <CANaxB-zLxs2=gNgWTqstLvyPK8mSwpEu2ob35TtaKWheMejZOQ@mail.gmail.com>
Subject: Re: [syzbot] [fs?] WARNING in pagemap_scan_pmd_entry
To:     Peter Xu <peterx@redhat.com>
Cc:     syzbot <syzbot+e94c5aaf7890901ebf9b@syzkaller.appspotmail.com>,
        Muhammad Usama Anjum <musamaanjum@gmail.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        syzkaller-bugs@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Nov 15, 2023 at 4:53=E2=80=AFPM Peter Xu <peterx@redhat.com> wrote:
>
> Hi, Andrei, Muhammad,
>
> I had a look (as it triggered the guard I added before..), and I think I
> know what happened.  So far I think it's a question to the new ioctl()
> interface, which I'd like to double check with you all.  See below.
>
> On Wed, Nov 15, 2023 at 01:07:18PM -0800, Andrei Vagin wrote:
> > Cc: Peter and Muhammad
> >
> > On Wed, Nov 15, 2023 at 6:41=E2=80=AFAM syzbot
> > <syzbot+e94c5aaf7890901ebf9b@syzkaller.appspotmail.com> wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following issue on:
> > >
> > > HEAD commit:    c42d9eeef8e5 Merge tag 'hardening-v6.7-rc2' of git://=
git.k..
> > > git tree:       upstream
> > > console+strace: https://syzkaller.appspot.com/x/log.txt?x=3D13626650e=
80000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=3D84217b7fc=
4acdc59
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=3De94c5aaf789=
0901ebf9b
> > > compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils f=
or Debian) 2.40
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=3D15d73be=
0e80000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=3D13670da8e=
80000
> > >
> > > Downloadable assets:
> > > disk image: https://storage.googleapis.com/syzbot-assets/a595d90eb9af=
/disk-c42d9eee.raw.xz
> > > vmlinux: https://storage.googleapis.com/syzbot-assets/c1e726fedb94/vm=
linux-c42d9eee.xz
> > > kernel image: https://storage.googleapis.com/syzbot-assets/cb43ae262d=
09/bzImage-c42d9eee.xz
> > >
> > > IMPORTANT: if you fix the issue, please add the following tag to the =
commit:
> > > Reported-by: syzbot+e94c5aaf7890901ebf9b@syzkaller.appspotmail.com
> > >
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 1 PID: 5071 at arch/x86/include/asm/pgtable.h:403 pte_u=
ffd_wp arch/x86/include/asm/pgtable.h:403 [inline]
>
> This is the guard I added to detect writable bit set even if uffd-wp bit =
is
> not yet cleared.  It means something obviously wrong happened.
>
> Here afaict the wrong thing is ioctl(PAGEMAP_SCAN) allows applying uffd-w=
p
> bit to VMA that is not even registered with userfault.  Then what happene=
d
> is when the page is written, do_wp_page() will try to reuse the anonymous
> page with the uffd-wp bit set, set W bit on top of it.

Thank you for looking at this.

>
> Below change works for me:
>
> =3D=3D=3D8<=3D=3D=3D
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index ef2eb12906da..8a2500fa4580 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1987,6 +1987,12 @@ static int pagemap_scan_test_walk(unsigned long st=
art, unsigned long end,
>                 vma_category |=3D PAGE_IS_WPALLOWED;
>         else if (p->arg.flags & PM_SCAN_CHECK_WPASYNC)
>                 return -EPERM;
> +       else
> +               /*
> +                * Neither has the VMA enabled WP tracking, nor does the
> +                * user want to explicit fail the walk.  Skip the vma.
> +                */
> +               return 1;

In this case, I think we need to check the PM_SCAN_WP_MATCHING flag
and skip these vma-s only if it is set.

If PM_SCAN_WP_MATCHING isn't set, this ioctl returns page flags and
can be used without the intention of tracking memory changes.

>
>         if (vma->vm_flags & VM_PFNMAP)
>                 return 1;
> =3D=3D=3D8<=3D=3D=3D
>
> This is based on my reading of the pagemap scan flags:
>
> - Write-protect the pages. The ``PM_SCAN_WP_MATCHING`` is used to write-p=
rotect
>   the pages of interest. The ``PM_SCAN_CHECK_WPASYNC`` aborts the operati=
on if
>   non-Async Write Protected pages are found. The ``PM_SCAN_WP_MATCHING`` =
can be
>   used with or without ``PM_SCAN_CHECK_WPASYNC``.
>
> If PM_SCAN_CHECK_WPASYNC is used to enforce the check, we need to skip th=
e
> vma that is not registered properly.  Does it look reasonable to you?

I think the idea here could be to report page flags but doesn't
write-protect such pages.

Thanks,
Andrei