[PATCH] aio: Do not return -ERESTARTSYS (and friends) from AIO

[PATCH] aio: Do not return -ERESTARTSYS (and friends) from AIO

[Jan Kara]

We must not leak ERESTARTSYS (and similar error codes) to userspace as a return
value of IO. Because other AIO could have been already submitted by the same
io_submit syscall, there is no easy way to restart the syscall. So we do not
have much other options than fail the particular IO with EINTR.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/aio.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

 Not sure who should merge it... Andrew?

diff --git a/fs/aio.c b/fs/aio.c
index 1ccf25c..66e2d42 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -712,8 +712,18 @@ static ssize_t aio_run_iocb(struct kiocb *iocb)
 	 */
 	ret = retry(iocb);
 
-	if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED)
+	if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED) {
+		/*
+		 * There's no easy way to restart the syscall since other AIO's
+		 * may be already running. Just fail this IO with EINTR.
+		 */
+		if (unlikely(ret == -ERESTARTSYS ||
+			     ret == -ERESTARTNOINTR ||
+			     ret == -ERESTARTNOHAND ||
+			     ret == -ERESTART_RESTARTBLOCK))
+			ret = -EINTR;
 		aio_complete(iocb, ret, 0);
+	}
 out:
 	spin_lock_irq(&ctx->ctx_lock);
 
-- 
1.6.4.2

Re: [PATCH] aio: Do not return -ERESTARTSYS (and friends) from AIO

[Jeff Moyer]

Jan Kara <jack@suse.cz> writes:

> We must not leak ERESTARTSYS (and similar error codes) to userspace as a return
> value of IO. Because other AIO could have been already submitted by the same
> io_submit syscall, there is no easy way to restart the syscall. So we do not
> have much other options than fail the particular IO with EINTR.

This is a confusing problem description.  First, I'm assuming you
actually experienced this, is that right?  Care to share the details of
that?

Next, assuming we can get ERSTARTSYS and friends, it will be the return
code of a single iocb (reaped via io_getevents), not the return code of
the io_submit system call.  I'm not saying this is right, I'm just
saying that your description of the problem is misleading.

Cheers,
Jeff

> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/aio.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
>
>  Not sure who should merge it... Andrew?
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 1ccf25c..66e2d42 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -712,8 +712,18 @@ static ssize_t aio_run_iocb(struct kiocb *iocb)
>  	 */
>  	ret = retry(iocb);
>  
> -	if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED)
> +	if (ret != -EIOCBRETRY && ret != -EIOCBQUEUED) {
> +		/*
> +		 * There's no easy way to restart the syscall since other AIO's
> +		 * may be already running. Just fail this IO with EINTR.
> +		 */
> +		if (unlikely(ret == -ERESTARTSYS ||
> +			     ret == -ERESTARTNOINTR ||
> +			     ret == -ERESTARTNOHAND ||
> +			     ret == -ERESTART_RESTARTBLOCK))
> +			ret = -EINTR;
>  		aio_complete(iocb, ret, 0);
> +	}
>  out:
>  	spin_lock_irq(&ctx->ctx_lock);