* [PATCH] userfaultfd: clear flag if remap event not enabled
@ 2018-12-10 6:51 Peter Xu
2018-12-10 17:51 ` Mike Rapoport
0 siblings, 1 reply; 4+ messages in thread
From: Peter Xu @ 2018-12-10 6:51 UTC (permalink / raw)
To: linux-kernel
Cc: peterx, Andrea Arcangeli, Andrew Morton, Mike Rapoport,
Kirill A . Shutemov, Hugh Dickins, Pavel Emelyanov,
Pravin Shedge, linux-mm
When the process being tracked do mremap() without
UFFD_FEATURE_EVENT_REMAP on the corresponding tracking uffd file
handle, we should not generate the remap event, and at the same
time we should clear all the uffd flags on the new VMA. Without
this patch, we can still have the VM_UFFD_MISSING|VM_UFFD_WP
flags on the new VMA even the fault handling process does not
even know the existance of the VMA.
CC: Andrea Arcangeli <aarcange@redhat.com>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Mike Rapoport <rppt@linux.vnet.ibm.com>
CC: Kirill A. Shutemov <kirill@shutemov.name>
CC: Hugh Dickins <hughd@google.com>
CC: Pavel Emelyanov <xemul@virtuozzo.com>
CC: Pravin Shedge <pravin.shedge4linux@gmail.com>
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
Signed-off-by: Peter Xu <peterx@redhat.com>
---
fs/userfaultfd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index cd58939dc977..798ae8a438ff 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -740,6 +740,9 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma,
vm_ctx->ctx = ctx;
userfaultfd_ctx_get(ctx);
WRITE_ONCE(ctx->mmap_changing, true);
+ } else if (ctx) {
+ vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
+ vma->vm_flags &= ~(VM_UFFD_WP | VM_UFFD_MISSING);
}
}
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] userfaultfd: clear flag if remap event not enabled
2018-12-10 6:51 [PATCH] userfaultfd: clear flag if remap event not enabled Peter Xu
@ 2018-12-10 17:51 ` Mike Rapoport
2018-12-10 20:09 ` Andrea Arcangeli
0 siblings, 1 reply; 4+ messages in thread
From: Mike Rapoport @ 2018-12-10 17:51 UTC (permalink / raw)
To: Peter Xu
Cc: linux-kernel, Andrea Arcangeli, Andrew Morton, Mike Rapoport,
Kirill A . Shutemov, Hugh Dickins, Pavel Emelyanov,
Pravin Shedge, linux-mm
On Mon, Dec 10, 2018 at 02:51:21PM +0800, Peter Xu wrote:
> When the process being tracked do mremap() without
> UFFD_FEATURE_EVENT_REMAP on the corresponding tracking uffd file
> handle, we should not generate the remap event, and at the same
> time we should clear all the uffd flags on the new VMA. Without
> this patch, we can still have the VM_UFFD_MISSING|VM_UFFD_WP
> flags on the new VMA even the fault handling process does not
> even know the existance of the VMA.
>
> CC: Andrea Arcangeli <aarcange@redhat.com>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Mike Rapoport <rppt@linux.vnet.ibm.com>
> CC: Kirill A. Shutemov <kirill@shutemov.name>
> CC: Hugh Dickins <hughd@google.com>
> CC: Pavel Emelyanov <xemul@virtuozzo.com>
> CC: Pravin Shedge <pravin.shedge4linux@gmail.com>
> CC: linux-mm@kvack.org
> CC: linux-kernel@vger.kernel.org
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> fs/userfaultfd.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index cd58939dc977..798ae8a438ff 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -740,6 +740,9 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma,
> vm_ctx->ctx = ctx;
> userfaultfd_ctx_get(ctx);
> WRITE_ONCE(ctx->mmap_changing, true);
> + } else if (ctx) {
> + vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> + vma->vm_flags &= ~(VM_UFFD_WP | VM_UFFD_MISSING);
My preference would be
if (!ctx)
return;
if (ctx->features & UFFD_FEATURE_EVENT_REMAP) {
...
} else {
...
}
but I don't feel strongly about it.
I'd appreciate a comment in the code and with it
Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> }
> }
>
> --
> 2.17.1
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] userfaultfd: clear flag if remap event not enabled
2018-12-10 17:51 ` Mike Rapoport
@ 2018-12-10 20:09 ` Andrea Arcangeli
2018-12-11 5:16 ` Peter Xu
0 siblings, 1 reply; 4+ messages in thread
From: Andrea Arcangeli @ 2018-12-10 20:09 UTC (permalink / raw)
To: Mike Rapoport
Cc: Peter Xu, linux-kernel, Andrew Morton, Mike Rapoport,
Kirill A . Shutemov, Hugh Dickins, Pavel Emelyanov,
Pravin Shedge, linux-mm
Hello,
On Mon, Dec 10, 2018 at 07:51:16PM +0200, Mike Rapoport wrote:
> On Mon, Dec 10, 2018 at 02:51:21PM +0800, Peter Xu wrote:
> > When the process being tracked do mremap() without
> > UFFD_FEATURE_EVENT_REMAP on the corresponding tracking uffd file
> > handle, we should not generate the remap event, and at the same
> > time we should clear all the uffd flags on the new VMA. Without
> > this patch, we can still have the VM_UFFD_MISSING|VM_UFFD_WP
> > flags on the new VMA even the fault handling process does not
> > even know the existance of the VMA.
> >
> > CC: Andrea Arcangeli <aarcange@redhat.com>
> > CC: Andrew Morton <akpm@linux-foundation.org>
> > CC: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > CC: Kirill A. Shutemov <kirill@shutemov.name>
> > CC: Hugh Dickins <hughd@google.com>
> > CC: Pavel Emelyanov <xemul@virtuozzo.com>
> > CC: Pravin Shedge <pravin.shedge4linux@gmail.com>
> > CC: linux-mm@kvack.org
> > CC: linux-kernel@vger.kernel.org
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > fs/userfaultfd.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > index cd58939dc977..798ae8a438ff 100644
> > --- a/fs/userfaultfd.c
> > +++ b/fs/userfaultfd.c
> > @@ -740,6 +740,9 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma,
> > vm_ctx->ctx = ctx;
> > userfaultfd_ctx_get(ctx);
> > WRITE_ONCE(ctx->mmap_changing, true);
> > + } else if (ctx) {
> > + vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> > + vma->vm_flags &= ~(VM_UFFD_WP | VM_UFFD_MISSING);
Great catch Peter!
>
> My preference would be
>
> if (!ctx)
> return;
>
> if (ctx->features & UFFD_FEATURE_EVENT_REMAP) {
> ...
> } else {
> ...
> }
>
> but I don't feel strongly about it.
Yes, it'd look nicer to run a single "ctx not null" check.
>
> I'd appreciate a comment in the code and with it
>
> Acked-by: Mike Rapoport <rppt@linux.ibm.com>
>
Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] userfaultfd: clear flag if remap event not enabled
2018-12-10 20:09 ` Andrea Arcangeli
@ 2018-12-11 5:16 ` Peter Xu
0 siblings, 0 replies; 4+ messages in thread
From: Peter Xu @ 2018-12-11 5:16 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Mike Rapoport, linux-kernel, Andrew Morton, Mike Rapoport,
Kirill A . Shutemov, Hugh Dickins, Pavel Emelyanov,
Pravin Shedge, linux-mm
On Mon, Dec 10, 2018 at 03:09:25PM -0500, Andrea Arcangeli wrote:
> Hello,
>
> On Mon, Dec 10, 2018 at 07:51:16PM +0200, Mike Rapoport wrote:
> > On Mon, Dec 10, 2018 at 02:51:21PM +0800, Peter Xu wrote:
> > > When the process being tracked do mremap() without
> > > UFFD_FEATURE_EVENT_REMAP on the corresponding tracking uffd file
> > > handle, we should not generate the remap event, and at the same
> > > time we should clear all the uffd flags on the new VMA. Without
> > > this patch, we can still have the VM_UFFD_MISSING|VM_UFFD_WP
> > > flags on the new VMA even the fault handling process does not
> > > even know the existance of the VMA.
> > >
> > > CC: Andrea Arcangeli <aarcange@redhat.com>
> > > CC: Andrew Morton <akpm@linux-foundation.org>
> > > CC: Mike Rapoport <rppt@linux.vnet.ibm.com>
> > > CC: Kirill A. Shutemov <kirill@shutemov.name>
> > > CC: Hugh Dickins <hughd@google.com>
> > > CC: Pavel Emelyanov <xemul@virtuozzo.com>
> > > CC: Pravin Shedge <pravin.shedge4linux@gmail.com>
> > > CC: linux-mm@kvack.org
> > > CC: linux-kernel@vger.kernel.org
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> > > ---
> > > fs/userfaultfd.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > > index cd58939dc977..798ae8a438ff 100644
> > > --- a/fs/userfaultfd.c
> > > +++ b/fs/userfaultfd.c
> > > @@ -740,6 +740,9 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma,
> > > vm_ctx->ctx = ctx;
> > > userfaultfd_ctx_get(ctx);
> > > WRITE_ONCE(ctx->mmap_changing, true);
> > > + } else if (ctx) {
> > > + vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> > > + vma->vm_flags &= ~(VM_UFFD_WP | VM_UFFD_MISSING);
>
> Great catch Peter!
>
> >
> > My preference would be
> >
> > if (!ctx)
> > return;
> >
> > if (ctx->features & UFFD_FEATURE_EVENT_REMAP) {
> > ...
> > } else {
> > ...
> > }
> >
> > but I don't feel strongly about it.
>
> Yes, it'd look nicer to run a single "ctx not null" check.
I agree.
>
> >
> > I'd appreciate a comment in the code and with it
> >
> > Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> >
>
> Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Thanks to both! I'll repost soon.
Regards,
--
Peter Xu
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-11 5:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-10 6:51 [PATCH] userfaultfd: clear flag if remap event not enabled Peter Xu
2018-12-10 17:51 ` Mike Rapoport
2018-12-10 20:09 ` Andrea Arcangeli
2018-12-11 5:16 ` Peter Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox