Re: [PATCH 3/8] aio_poll(): sanitize the logics after vfs_poll(), get rid of leak on error

[zhengbin (A)]

+	if (async && !apt.error)  --->may be this should be if (!async && !apt.error) ?

On 2019/3/7 8:03, Al Viro wrote:
> From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> 
> We want iocb_put() happening on errors, to balance the extra reference
> we'd taken.  As it is, we end up with a leak.  The rules should be
> 	* error: iocb_put() to deal with the extra ref, return error,
> let the caller do another iocb_put().
> 	* async: iocb_put() to deal with the extra ref, return 0.
> 	* no error, event present immediately: aio_poll_complete() to
> report it, iocb_put() to deal with the extra ref, return 0.
> 
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
>  fs/aio.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index 3a8b894378e0..22b288997441 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1724,6 +1724,7 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, const struct iocb *iocb)
>  	struct kioctx *ctx = aiocb->ki_ctx;
>  	struct poll_iocb *req = &aiocb->poll;
>  	struct aio_poll_table apt;
> +	bool async = false;
>  	__poll_t mask;
>  
>  	/* reject any unknown events outside the normal event mask. */
> @@ -1760,30 +1761,26 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, const struct iocb *iocb)
>  
>  	spin_lock_irq(&ctx->ctx_lock);
>  	spin_lock(&req->head->lock);
> -	if (req->woken) {
> -		/* wake_up context handles the rest */
> -		mask = 0;
> +	if (req->woken) { /* already taken up by aio_poll_wake() */
> +		async = true;
>  		apt.error = 0;
> -	} else if (mask || apt.error) {
> -		/* if we get an error or a mask we are done */
> -		WARN_ON_ONCE(list_empty(&req->wait.entry));
> -		list_del_init(&req->wait.entry);
> -	} else {
> -		/* actually waiting for an event */
> +	} else if (!mask && !apt.error) { /* actually waiting for an event */
>  		list_add_tail(&aiocb->ki_list, &ctx->active_reqs);
>  		aiocb->ki_cancel = aio_poll_cancel;
> +		async = true;
> +	} else { /* if we get an error or a mask we are done */
> +		WARN_ON_ONCE(list_empty(&req->wait.entry));
> +		list_del_init(&req->wait.entry);
> +		/* no wakeup in the future either; aiocb is ours to dispose of */
>  	}
>  	spin_unlock(&req->head->lock);
>  	spin_unlock_irq(&ctx->ctx_lock);
>  
>  out:
> -	if (unlikely(apt.error))
> -		return apt.error;
> -
> -	if (mask)
> +	if (async && !apt.error)
>  		aio_poll_complete(aiocb, mask);
>  	iocb_put(aiocb);
> -	return 0;
> +	return apt.error;
>  }
>  
>  static int __io_submit_one(struct kioctx *ctx, const struct iocb *iocb,
>