[PATCH] io_uring: fix submission-failure handling for uring-cmd

[PATCH] io_uring: fix submission-failure handling for uring-cmd

[Kanchan Joshi]

If ->uring_cmd returned an error value different from -EAGAIN or
-EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble
as caller (io_uring core code) handles IOU_OK differently than other
error codes.
Fix this by returning the actual error code.

Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
---
 io_uring/uring_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index b0e7feeed365..6f99dbd5d550 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -119,7 +119,7 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
 		if (ret < 0)
 			req_set_fail(req);
 		io_req_set_res(req, ret, 0);
-		return IOU_OK;
+		return ret;
 	}
 
 	return IOU_ISSUE_SKIP_COMPLETE;
-- 
2.25.1

Re: [PATCH] io_uring: fix submission-failure handling for uring-cmd

[Jens Axboe]

On 8/23/22 9:10 AM, Kanchan Joshi wrote:
> If ->uring_cmd returned an error value different from -EAGAIN or
> -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble
> as caller (io_uring core code) handles IOU_OK differently than other
> error codes.
> Fix this by returning the actual error code.

Not sure if this is strictly needed, as the cqe error is set just
fine. But I guess some places also check return value of the issue
path.

Applied.

-- 
Jens Axboe

Re: [PATCH] io_uring: fix submission-failure handling for uring-cmd

[Kanchan Joshi]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On Tue, Aug 23, 2022 at 09:47:39AM -0600, Jens Axboe wrote:
>On 8/23/22 9:10 AM, Kanchan Joshi wrote:
>> If ->uring_cmd returned an error value different from -EAGAIN or
>> -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble
>> as caller (io_uring core code) handles IOU_OK differently than other
>> error codes.
>> Fix this by returning the actual error code.
>
>Not sure if this is strictly needed, as the cqe error is set just
>fine. But I guess some places also check return value of the issue
>path.

So I was testing iopoll support and ran into this issue - submission
failed (expected one), control came back to this point, error code
got converted to IOU_OK, and it started polling endlessly for a command
that never got submitted.
io_issue_sqe continued to invoke io_iopoll_req_issued() rather than
bailing out.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] io_uring: fix submission-failure handling for uring-cmd

[Jens Axboe]

On 8/23/22 10:47, Kanchan Joshi wrote:
> On Tue, Aug 23, 2022 at 09:47:39AM -0600, Jens Axboe wrote:
>> On 8/23/22 9:10 AM, Kanchan Joshi wrote:
>>> If ->uring_cmd returned an error value different from -EAGAIN or
>>> -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble
>>> as caller (io_uring core code) handles IOU_OK differently than other
>>> error codes.
>>> Fix this by returning the actual error code.
>>
>> Not sure if this is strictly needed, as the cqe error is set just
>> fine. But I guess some places also check return value of the issue
>> path.
> 
> So I was testing iopoll support and ran into this issue - submission
> failed (expected one), control came back to this point, error code
> got converted to IOU_OK, and it started polling endlessly for a command
> that never got submitted.
> io_issue_sqe continued to invoke io_iopoll_req_issued() rather than
> bailing out.

Ah ok, yes for iopoll it'd make a difference...

-- 
Jens Axboe