All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Benjamin LaHaise <bcrl@kvack.org>
Cc: linux-aio@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ramji Jiyani <ramjiyani@google.com>,
	Christoph Hellwig <hch@lst.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	stable@vger.kernel.org
Subject: Re: [PATCH 1/2] aio: keep poll requests on waitqueue until completed
Date: Mon, 6 Dec 2021 10:56:07 -0800	[thread overview]
Message-ID: <Ya5cx3EcU5SgV9dP@gmail.com> (raw)
In-Reply-To: <20211204002301.116139-2-ebiggers@kernel.org>

On Fri, Dec 03, 2021 at 04:23:00PM -0800, Eric Biggers wrote:
> @@ -1680,20 +1690,24 @@ static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
>  	if (mask && !(mask & req->events))
>  		return 0;
>  
> -	list_del_init(&req->wait.entry);
> -
> +	/*
> +	 * Complete the iocb inline if possible.  This requires that two
> +	 * conditions be met:
> +	 *   1. The event mask must have been passed.  If a regular wakeup was
> +	 *	done instead, then mask == 0 and we have to call vfs_poll() to
> +	 *	get the events, so inline completion isn't possible.
> +	 *   2. ctx_lock must not be busy.  We have to use trylock because we
> +	 *      already hold the waitqueue lock, so this inverts the normal
> +	 *      locking order.  Use irqsave/irqrestore because not all
> +	 *      filesystems (e.g. fuse) call this function with IRQs disabled,
> +	 *      yet IRQs have to be disabled before ctx_lock is obtained.
> +	 */
>  	if (mask && spin_trylock_irqsave(&iocb->ki_ctx->ctx_lock, flags)) {
>  		struct kioctx *ctx = iocb->ki_ctx;
>  
> -		/*
> -		 * Try to complete the iocb inline if we can. Use
> -		 * irqsave/irqrestore because not all filesystems (e.g. fuse)
> -		 * call this function with IRQs disabled and because IRQs
> -		 * have to be disabled before ctx_lock is obtained.
> -		 */
> +		list_del_init(&req->wait.entry);
>  		list_del(&iocb->ki_list);
>  		iocb->ki_res.res = mangle_poll(mask);
> -		req->done = true;
>  		if (iocb->ki_eventfd && eventfd_signal_allowed()) {
>  			iocb = NULL;
>  			INIT_WORK(&req->work, aio_poll_put_work);
> @@ -1703,7 +1717,16 @@ static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
>  		if (iocb)
>  			iocb_put(iocb);
>  	} else {

I think I missed something here.  Now that the request is left on the waitqueue,
there needs to be a third condition for completing the iocb inline: the
completion work must not have already been scheduled.

- Eric

  reply	other threads:[~2021-12-06 18:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-04  0:22 [PATCH 0/2] aio poll: fix use-after-free and missing wakeups Eric Biggers
2021-12-04  0:23 ` [PATCH 1/2] aio: keep poll requests on waitqueue until completed Eric Biggers
2021-12-06 18:56   ` Eric Biggers [this message]
2021-12-04  0:23 ` [PATCH 2/2] aio: fix use-after-free due to missing POLLFREE handling Eric Biggers
2021-12-06 19:28   ` Linus Torvalds
2021-12-06 19:54     ` Eric Biggers
2021-12-06 21:48       ` Linus Torvalds
2021-12-07 10:21         ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Ya5cx3EcU5SgV9dP@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=bcrl@kvack.org \
    --cc=hch@lst.de \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=ramjiyani@google.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.