linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* userfaultfd use-after-free
@ 2017-09-19 13:18 Mark Rutland
  2017-09-20 18:04 ` [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free Andrea Arcangeli
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Rutland @ 2017-09-19 13:18 UTC (permalink / raw)
  To: Andrea Arcangeli, Pavel Emelyanov, Mike Rapoport
  Cc: linux-kernel, linux-fsdevel, linux-mm, Andrew Morton,
	Mike Kravetz, syzkaller

Hi,

Syzkaller found a use-after-free in the userfaultfd code, which is very
easy to trigger on x86_64 and arm64 in v4.13 and v4.14-rc1. I believe
this may go as far back as v4.10, when fork events were introduced.

While fuzzing I've been seeing some other intermittent memory corruption bugs
which I believe are related -- I only tracked this down to userfaultfd after
enabling both KASAN and DEBUG_LIST.

I've included an example log, a (hopefully legible) C reproducer, and Syzkaller
reproducer below. These can also be found on my kernel.org web space, along
with other logs, configs, etc:

  https://www.kernel.org/pub/linux/kernel/people/mark/bugs/20170919-userfaultfd-uaf/

Thanks,
Mark.

arm64 log (v4.13)
----
[    3.286772] ==================================================================
[    3.288470] BUG: KASAN: use-after-free in __list_del_entry_valid+0x148/0x188
[    3.290230] Read of size 8 at addr ffff80000af53b40 by task repro/1374
[    3.291682]
[    3.292099] CPU: 2 PID: 1374 Comm: repro Not tainted 4.13.0 #47
[    3.293653] Hardware name: linux,dummy-virt (DT)
[    3.294862] Call trace:
[    3.295506] [<ffff20000808fd00>] dump_backtrace+0x0/0x420
[    3.296887] [<ffff2000080903ec>] show_stack+0x14/0x20
[    3.298173] [<ffff2000098c1424>] dump_stack+0xcc/0xf8
[    3.299463] [<ffff2000083dc2c0>] print_address_description+0x60/0x250
[    3.301101] [<ffff2000083dc7b0>] kasan_report+0x238/0x2f8
[    3.302474] [<ffff2000083dc8e8>] __asan_report_load8_noabort+0x18/0x20
[    3.304144] [<ffff2000088d74a0>] __list_del_entry_valid+0x148/0x188
[    3.305734] [<ffff2000084dc5d8>] userfaultfd_event_wait_completion+0x278/0x568
[    3.307567] [<ffff2000084e0f38>] dup_userfaultfd_complete+0x110/0x290
[    3.309205] [<ffff200008114df4>] copy_process.isra.6.part.7+0x39b4/0x4768
[    3.310920] [<ffff200008115f60>] _do_fork+0x120/0x590
[    3.312209] [<ffff200008116498>] SyS_clone+0x18/0x20
[    3.313471] [<ffff200008083f30>] el0_svc_naked+0x24/0x28
[    3.314816]
[    3.315212] The buggy address belongs to the page:
[    3.316439] page:ffff7e00002bd4c0 count:0 mapcount:0 mapping:          (null) index:0x0
[    3.318456] flags: 0xfffc00000000000()
[    3.319208] raw: 0fffc00000000000 0000000000000000 0000000000000000 00000000ffffffff
[    3.321177] raw: dead000000000100 dead000000000200 0000000000000000 0000000000000000
[    3.323125] page dumped because: kasan: bad access detected
[    3.324542]
[    3.324938] Memory state around the buggy address:
[    3.326155]  ffff80000af53a00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    3.327983]  ffff80000af53a80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    3.329808] >ffff80000af53b00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    3.331635]                                            ^
[    3.332980]  ffff80000af53b80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    3.334801]  ffff80000af53c00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[    3.336627] ==================================================================
[    3.338447] Disabling lock debugging due to kernel taint
[    3.339915] Kernel panic - not syncing: panic_on_warn set ...
[    3.339915]
[    3.341081] CPU: 2 PID: 1374 Comm: repro Tainted: G    B           4.13.0 #47
[    3.342884] Hardware name: linux,dummy-virt (DT)
[    3.344062] Call trace:
[    3.344698] [<ffff20000808fd00>] dump_backtrace+0x0/0x420
[    3.346066] [<ffff2000080903ec>] show_stack+0x14/0x20
[    3.347346] [<ffff2000098c1424>] dump_stack+0xcc/0xf8
[    3.348637] [<ffff2000081179c4>] panic+0x1e4/0x358
[    3.349855] [<ffff2000083dc230>] kasan_save_enable_multi_shot+0x0/0x30
[    3.351504] [<ffff2000083dc66c>] kasan_report+0xf4/0x2f8
[    3.352860] [<ffff2000083dc8e8>] __asan_report_load8_noabort+0x18/0x20
[    3.354509] [<ffff2000088d74a0>] __list_del_entry_valid+0x148/0x188
[    3.356101] [<ffff2000084dc5d8>] userfaultfd_event_wait_completion+0x278/0x568
[    3.357920] [<ffff2000084e0f38>] dup_userfaultfd_complete+0x110/0x290
[    3.359553] [<ffff200008114df4>] copy_process.isra.6.part.7+0x39b4/0x4768
[    3.361267] [<ffff200008115f60>] _do_fork+0x120/0x590
[    3.362549] [<ffff200008116498>] SyS_clone+0x18/0x20
[    3.363815] [<ffff200008083f30>] el0_svc_naked+0x24/0x28
[    3.365161] SMP: stopping secondary CPUs
[    3.366180] Kernel Offset: disabled
[    3.366784] CPU features: 0x002082
[    3.367362] Memory Limit: none
[    3.367897] Rebooting in 86400 seconds..
----

Syzkaller reproducer
----
mmap(&(0x7f0000000000/0xc72000)=nil, 0xc72000, 0x3, 0x32, 0xffffffffffffffff, 0x0)
r0 = userfaultfd(0x0)
ioctl$UFFDIO_API(r0, 0xc018aa3f, &(0x7f0000c08000-0x18)={0xaa, 0x2, 0x0})
setrlimit(0x7, &(0x7f0000000000)={0x0, 0x0})
ioctl$UFFDIO_REGISTER(r0, 0xc020aa00, &(0x7f0000010000)={{&(0x7f0000007000/0x3000)=nil, 0x3000}, 0x1, 0x0})
read(r0, &(0x7f0000015000-0x1000)="0000000000000000000000000000000000000000000000000000000000000000", 0x20)
clone(0x1000000, &(0x7f0000006000)="", &(0x7f0000012000)=0x0, &(0x7f0000014000-0x4)=0x0, &(0x7f0000011000-0x25)="")
----

C reproducer
----
#include <linux/userfaultfd.h>
#include <pthread.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/resource.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>

int userfaultfd(int flags)
{
	return syscall(__NR_userfaultfd, flags);
}

// Only exists since v4.10, so not in most distro headers...
#ifndef UFFD_FEATURE_EVENT_FORK
#define UFFD_FEATURE_EVENT_FORK 2
#endif

// Arbitrary; needs to be some PAGE_SIZE multiple
#define REGION_SIZE	(2 * 1024 * 1024)

int uffd_setup(void *base, size_t size)
{
	int uffd;
	struct uffdio_api ufa = {
		.api = UFFD_API,
		.features = UFFD_FEATURE_EVENT_FORK,
		.ioctls = 0,
	};
	struct uffdio_register ufr = {
		.range.start = (unsigned long)base,
		.range.len = size,
		.mode = UFFDIO_REGISTER_MODE_MISSING,
		.ioctls = 0,
	};
	
	uffd = userfaultfd(0);
	ioctl(uffd, UFFDIO_API, &ufa);
	ioctl(uffd, UFFDIO_REGISTER, &ufr);

	return uffd;
}

void *thr_uffd(void *unused)
{
	void *base;
	int uffd;
	struct uffd_msg msg;
	struct rlimit rlimit;

	base = mmap(NULL, REGION_SIZE, PROT_READ | PROT_WRITE,
		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);

	uffd = uffd_setup(base, REGION_SIZE);

	rlimit.rlim_cur = 0;
	rlimit.rlim_max = 0;
	setrlimit(RLIMIT_NOFILE, &rlimit);

	read(uffd, &msg, sizeof(msg));

	return NULL;
}

void *thr_clone(void *unused)
{
	fork();
	return NULL;
}

int main(int argc, char *argv[])
{
	pthread_t p_uffd, p_clone;

	pthread_create(&p_uffd, 0, thr_uffd, NULL);
	usleep(1000);
	pthread_create(&p_clone, 0, thr_clone, NULL);
	usleep(1000);

	return 0;
}
----

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free
  2017-09-19 13:18 userfaultfd use-after-free Mark Rutland
@ 2017-09-20 18:04 ` Andrea Arcangeli
  2017-09-20 18:30   ` Greg KH
  2017-09-21 10:46   ` Mark Rutland
  0 siblings, 2 replies; 4+ messages in thread
From: Andrea Arcangeli @ 2017-09-20 18:04 UTC (permalink / raw)
  To: Andrew Morton, Mark Rutland
  Cc: Pavel Emelyanov, Mike Rapoport, Dr. David Alan Gilbert,
	Mike Kravetz, linux-kernel, linux-fsdevel, linux-mm, syzkaller,
	stable

When reading the event from the uffd, we put it on a temporary
fork_event list to detect if we can still access it after releasing
and retaking the event_wqh.lock.

If fork aborts and removes the event from the fork_event all is fine
as long as we're still in the userfault read context and fork_event
head is still alive.

We've to put the event allocated in the fork kernel stack, back from
fork_event list-head to the event_wqh head, before returning from
userfaultfd_ctx_read, because the fork_event head lifetime is limited
to the userfaultfd_ctx_read stack lifetime.

Forgetting to move the event back to its event_wqh place then results
in __remove_wait_queue(&ctx->event_wqh, &ewq->wq); in
userfaultfd_event_wait_completion to remove it from a head that has
been already freed from the reader stack.

This could only happen if resolve_userfault_fork failed (for example
if there are no file descriptors available to allocate the fork
uffd). If it succeeded it was put back correctly.

Furthermore, after find_userfault_evt receives a fork event, the
forked userfault context in fork_nctx and
uwq->msg.arg.reserved.reserved1 can be released by the fork thread as
soon as the event_wqh.lock is released. Taking a reference on the
fork_nctx before dropping the lock prevents an use after free in
resolve_userfault_fork().

If the fork side aborted and it already released everything, we still
try to succeed resolve_userfault_fork(), if possible.

Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
---
 fs/userfaultfd.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 10 deletions(-)

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 06d6cfda1e8e..16366587e579 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -599,6 +599,12 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
 			break;
 		if (ACCESS_ONCE(ctx->released) ||
 		    fatal_signal_pending(current)) {
+			/*
+			 * &ewq->wq may be queued in fork_event, but
+			 * __remove_wait_queue ignores the head
+			 * parameter. It would be a problem if it
+			 * didn't.
+			 */
 			__remove_wait_queue(&ctx->event_wqh, &ewq->wq);
 			if (ewq->msg.event == UFFD_EVENT_FORK) {
 				struct userfaultfd_ctx *new;
@@ -1072,6 +1078,12 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
 					(unsigned long)
 					uwq->msg.arg.reserved.reserved1;
 				list_move(&uwq->wq.entry, &fork_event);
+				/*
+				 * fork_nctx can be freed as soon as
+				 * we drop the lock, unless we take a
+				 * reference on it.
+				 */
+				userfaultfd_ctx_get(fork_nctx);
 				spin_unlock(&ctx->event_wqh.lock);
 				ret = 0;
 				break;
@@ -1102,19 +1114,53 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
 
 	if (!ret && msg->event == UFFD_EVENT_FORK) {
 		ret = resolve_userfault_fork(ctx, fork_nctx, msg);
+		spin_lock(&ctx->event_wqh.lock);
+		if (!list_empty(&fork_event)) {
+			/*
+			 * The fork thread didn't abort, so we can
+			 * drop the temporary refcount.
+			 */
+			userfaultfd_ctx_put(fork_nctx);
+
+			uwq = list_first_entry(&fork_event,
+					       typeof(*uwq),
+					       wq.entry);
+			/*
+			 * If fork_event list wasn't empty and in turn
+			 * the event wasn't already released by fork
+			 * (the event is allocated on fork kernel
+			 * stack), put the event back to its place in
+			 * the event_wq. fork_event head will be freed
+			 * as soon as we return so the event cannot
+			 * stay queued there no matter the current
+			 * "ret" value.
+			 */
+			list_del(&uwq->wq.entry);
+			__add_wait_queue(&ctx->event_wqh, &uwq->wq);
 
-		if (!ret) {
-			spin_lock(&ctx->event_wqh.lock);
-			if (!list_empty(&fork_event)) {
-				uwq = list_first_entry(&fork_event,
-						       typeof(*uwq),
-						       wq.entry);
-				list_del(&uwq->wq.entry);
-				__add_wait_queue(&ctx->event_wqh, &uwq->wq);
+			/*
+			 * Leave the event in the waitqueue and report
+			 * error to userland if we failed to resolve
+			 * the userfault fork.
+			 */
+			if (likely(!ret))
 				userfaultfd_event_complete(ctx, uwq);
-			}
-			spin_unlock(&ctx->event_wqh.lock);
+		} else {
+			/*
+			 * Here the fork thread aborted and the
+			 * refcount from the fork thread on fork_nctx
+			 * has already been released. We still hold
+			 * the reference we took before releasing the
+			 * lock above. If resolve_userfault_fork
+			 * failed we've to drop it because the
+			 * fork_nctx has to be freed in such case. If
+			 * it succeeded we'll hold it because the new
+			 * uffd references it.
+			 */
+			if (ret)
+				userfaultfd_ctx_put(fork_nctx);
 		}
+		spin_unlock(&ctx->event_wqh.lock);
 	}
 
 	return ret;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free
  2017-09-20 18:04 ` [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free Andrea Arcangeli
@ 2017-09-20 18:30   ` Greg KH
  2017-09-21 10:46   ` Mark Rutland
  1 sibling, 0 replies; 4+ messages in thread
From: Greg KH @ 2017-09-20 18:30 UTC (permalink / raw)
  To: Andrea Arcangeli
  Cc: Andrew Morton, Mark Rutland, Pavel Emelyanov, Mike Rapoport,
	Dr. David Alan Gilbert, Mike Kravetz, linux-kernel, linux-fsdevel,
	linux-mm, syzkaller, stable

On Wed, Sep 20, 2017 at 08:04:13PM +0200, Andrea Arcangeli wrote:
> When reading the event from the uffd, we put it on a temporary
> fork_event list to detect if we can still access it after releasing
> and retaking the event_wqh.lock.
> 
> If fork aborts and removes the event from the fork_event all is fine
> as long as we're still in the userfault read context and fork_event
> head is still alive.
> 
> We've to put the event allocated in the fork kernel stack, back from
> fork_event list-head to the event_wqh head, before returning from
> userfaultfd_ctx_read, because the fork_event head lifetime is limited
> to the userfaultfd_ctx_read stack lifetime.
> 
> Forgetting to move the event back to its event_wqh place then results
> in __remove_wait_queue(&ctx->event_wqh, &ewq->wq); in
> userfaultfd_event_wait_completion to remove it from a head that has
> been already freed from the reader stack.
> 
> This could only happen if resolve_userfault_fork failed (for example
> if there are no file descriptors available to allocate the fork
> uffd). If it succeeded it was put back correctly.
> 
> Furthermore, after find_userfault_evt receives a fork event, the
> forked userfault context in fork_nctx and
> uwq->msg.arg.reserved.reserved1 can be released by the fork thread as
> soon as the event_wqh.lock is released. Taking a reference on the
> fork_nctx before dropping the lock prevents an use after free in
> resolve_userfault_fork().
> 
> If the fork side aborted and it already released everything, we still
> try to succeed resolve_userfault_fork(), if possible.
> 
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> ---
>  fs/userfaultfd.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 56 insertions(+), 10 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free
  2017-09-20 18:04 ` [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free Andrea Arcangeli
  2017-09-20 18:30   ` Greg KH
@ 2017-09-21 10:46   ` Mark Rutland
  1 sibling, 0 replies; 4+ messages in thread
From: Mark Rutland @ 2017-09-21 10:46 UTC (permalink / raw)
  To: Andrea Arcangeli
  Cc: Andrew Morton, Pavel Emelyanov, Mike Rapoport,
	Dr. David Alan Gilbert, Mike Kravetz, linux-kernel, linux-fsdevel,
	linux-mm, syzkaller, stable

On Wed, Sep 20, 2017 at 08:04:13PM +0200, Andrea Arcangeli wrote:
> When reading the event from the uffd, we put it on a temporary
> fork_event list to detect if we can still access it after releasing
> and retaking the event_wqh.lock.
> 
> If fork aborts and removes the event from the fork_event all is fine
> as long as we're still in the userfault read context and fork_event
> head is still alive.
> 
> We've to put the event allocated in the fork kernel stack, back from
> fork_event list-head to the event_wqh head, before returning from
> userfaultfd_ctx_read, because the fork_event head lifetime is limited
> to the userfaultfd_ctx_read stack lifetime.
> 
> Forgetting to move the event back to its event_wqh place then results
> in __remove_wait_queue(&ctx->event_wqh, &ewq->wq); in
> userfaultfd_event_wait_completion to remove it from a head that has
> been already freed from the reader stack.
> 
> This could only happen if resolve_userfault_fork failed (for example
> if there are no file descriptors available to allocate the fork
> uffd). If it succeeded it was put back correctly.
> 
> Furthermore, after find_userfault_evt receives a fork event, the
> forked userfault context in fork_nctx and
> uwq->msg.arg.reserved.reserved1 can be released by the fork thread as
> soon as the event_wqh.lock is released. Taking a reference on the
> fork_nctx before dropping the lock prevents an use after free in
> resolve_userfault_fork().
> 
> If the fork side aborted and it already released everything, we still
> try to succeed resolve_userfault_fork(), if possible.
> 
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>

This has survived my test-case overnight, so FWIW:

Tested-by: Mark Rutland <mark.rutland@arm.com>

So that this can be backported to stable trees, I think we also need:

Fixes: 893e26e61d04eac9 ("userfaultfd: non-cooperative: Add fork() event")
Cc: <stable@vger.kernel.org>

Thanks,
Mark.

> ---
>  fs/userfaultfd.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 56 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 06d6cfda1e8e..16366587e579 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -599,6 +599,12 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
>  			break;
>  		if (ACCESS_ONCE(ctx->released) ||
>  		    fatal_signal_pending(current)) {
> +			/*
> +			 * &ewq->wq may be queued in fork_event, but
> +			 * __remove_wait_queue ignores the head
> +			 * parameter. It would be a problem if it
> +			 * didn't.
> +			 */
>  			__remove_wait_queue(&ctx->event_wqh, &ewq->wq);
>  			if (ewq->msg.event == UFFD_EVENT_FORK) {
>  				struct userfaultfd_ctx *new;
> @@ -1072,6 +1078,12 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
>  					(unsigned long)
>  					uwq->msg.arg.reserved.reserved1;
>  				list_move(&uwq->wq.entry, &fork_event);
> +				/*
> +				 * fork_nctx can be freed as soon as
> +				 * we drop the lock, unless we take a
> +				 * reference on it.
> +				 */
> +				userfaultfd_ctx_get(fork_nctx);
>  				spin_unlock(&ctx->event_wqh.lock);
>  				ret = 0;
>  				break;
> @@ -1102,19 +1114,53 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
>  
>  	if (!ret && msg->event == UFFD_EVENT_FORK) {
>  		ret = resolve_userfault_fork(ctx, fork_nctx, msg);
> +		spin_lock(&ctx->event_wqh.lock);
> +		if (!list_empty(&fork_event)) {
> +			/*
> +			 * The fork thread didn't abort, so we can
> +			 * drop the temporary refcount.
> +			 */
> +			userfaultfd_ctx_put(fork_nctx);
> +
> +			uwq = list_first_entry(&fork_event,
> +					       typeof(*uwq),
> +					       wq.entry);
> +			/*
> +			 * If fork_event list wasn't empty and in turn
> +			 * the event wasn't already released by fork
> +			 * (the event is allocated on fork kernel
> +			 * stack), put the event back to its place in
> +			 * the event_wq. fork_event head will be freed
> +			 * as soon as we return so the event cannot
> +			 * stay queued there no matter the current
> +			 * "ret" value.
> +			 */
> +			list_del(&uwq->wq.entry);
> +			__add_wait_queue(&ctx->event_wqh, &uwq->wq);
>  
> -		if (!ret) {
> -			spin_lock(&ctx->event_wqh.lock);
> -			if (!list_empty(&fork_event)) {
> -				uwq = list_first_entry(&fork_event,
> -						       typeof(*uwq),
> -						       wq.entry);
> -				list_del(&uwq->wq.entry);
> -				__add_wait_queue(&ctx->event_wqh, &uwq->wq);
> +			/*
> +			 * Leave the event in the waitqueue and report
> +			 * error to userland if we failed to resolve
> +			 * the userfault fork.
> +			 */
> +			if (likely(!ret))
>  				userfaultfd_event_complete(ctx, uwq);
> -			}
> -			spin_unlock(&ctx->event_wqh.lock);
> +		} else {
> +			/*
> +			 * Here the fork thread aborted and the
> +			 * refcount from the fork thread on fork_nctx
> +			 * has already been released. We still hold
> +			 * the reference we took before releasing the
> +			 * lock above. If resolve_userfault_fork
> +			 * failed we've to drop it because the
> +			 * fork_nctx has to be freed in such case. If
> +			 * it succeeded we'll hold it because the new
> +			 * uffd references it.
> +			 */
> +			if (ret)
> +				userfaultfd_ctx_put(fork_nctx);
>  		}
> +		spin_unlock(&ctx->event_wqh.lock);
>  	}
>  
>  	return ret;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-09-21 10:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-19 13:18 userfaultfd use-after-free Mark Rutland
2017-09-20 18:04 ` [PATCH 1/1] userfaultfd: non-cooperative: fix fork use after free Andrea Arcangeli
2017-09-20 18:30   ` Greg KH
2017-09-21 10:46   ` Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).