[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z880ejmfqjY1cuX7@x1.local>
Date: Mon, 10 Mar 2025 14:50:34 -0400
From: Peter Xu <peterx@...hat.com>
To: Jinjiang Tu <tujinjiang@...wei.com>
Cc: jimsiak <jimsiak@...ab.ece.ntua.gr>, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, viro@...iv.linux.org.uk,
linux-mm@...ck.org, wangkefeng.wang@...wei.com
Subject: Re: Using userfaultfd with KVM's async page fault handling causes
processes to hung waiting for mmap_lock to be released
On Mon, Mar 10, 2025 at 02:40:35PM +0800, Jinjiang Tu wrote:
>
> 在 2025/3/8 6:41, Peter Xu 写道:
> > On Fri, Mar 07, 2025 at 03:11:09PM +0200, jimsiak wrote:
> > > Hi,
> > >
> > > From my side, I managed to avoid the freezing of processes with the
> > > following change in function userfaultfd_release() in file fs/userfaultfd.c
> > > (https://elixir.bootlin.com/linux/v5.13/source/fs/userfaultfd.c#L842):
> > >
> > > I moved the following command from line 851:
> > > WRITE_ONCE(ctx->released, true);
> > > (https://elixir.bootlin.com/linux/v5.13/source/fs/userfaultfd.c#L851)
> > >
> > > to line 905, that is exactly before the functions returns 0.
> > >
> > > That simple workaround worked for my use case but I am far from sure that is
> > > a correct/sufficient fix for the problem at hand.
> > Updating the field after userfaultfd_ctx_put() might mean UAF, afaict.
> >
> > Maybe it's possible to remove ctx->released but only rely on the mmap write
> > lock. However that'll need some closer look and more thoughts.
> >
> > To me, the more straightforward way to fix it is to use the patch I
> > mentioned in the other email:
> >
> > https://lore.kernel.org/all/ZLmT3BfcmltfFvbq@x1n/
> >
> > Or does it mean it didn't work at all?
>
> This patch works for me. mlock() syscall calls GUP with FOLL_UNLOCKABLE and
> allows to release mmap lock and retry.
>
> But other GUP call without FOLL_UNLOCKABLE will return VM_FAULT_SIGBUS,
> is it a regression for the below commit?
Do you have an explicit reproducer / use case of such?
AFAIU, below commit should only change it from SIGBUS to NOPAGE when
"released" is set. I don't see how it can regress on !FOLL_UNLOCKABLE.
Thanks,
>
> commit 656710a60e3693911bee3a355d2f2bbae3faba33
> Author: Andrea Arcangeli <aarcange@...hat.com>
> Date: Fri Sep 8 16:12:42 2017 -0700
>
> userfaultfd: non-cooperative: closing the uffd without triggering SIGBUS
>
> >
> > Thanks,
> >
>
--
Peter Xu
Powered by blists - more mailing lists