Re: [PATCH 03/16] io_uring: make io_buffer_select() return the user address directly

[Jens Axboe <axboe@kernel.dk>]

On 5/9/22 6:12 AM, Dylan Yudaken wrote:
> On Mon, 2022-05-09 at 12:06 +0000, Dylan Yudaken wrote:
>> On Sun, 2022-05-01 at 14:56 -0600, Jens Axboe wrote:
>>> There's no point in having callers provide a kbuf, we're just
>>> returning
>>> the address anyway.
>>>
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>> ---
>>>  fs/io_uring.c | 42 ++++++++++++++++++------------------------
>>>  1 file changed, 18 insertions(+), 24 deletions(-)
>>>
>>
>> ...
>>
>>> @@ -6013,10 +6006,11 @@ static int io_recv(struct io_kiocb *req,
>>> unsigned int issue_flags)
>>>                 return -ENOTSOCK;
>>>  
>>>         if (req->flags & REQ_F_BUFFER_SELECT) {
>>> -               kbuf = io_buffer_select(req, &sr->len, sr->bgid,
>>> issue_flags);
>>> -               if (IS_ERR(kbuf))
>>> -                       return PTR_ERR(kbuf);
>>> -               buf = u64_to_user_ptr(kbuf->addr);
>>> +               void __user *buf;
>>
>> this now shadows the outer buf, and so does not work at all as the buf
>> value is lost.
>> A bit surprised this did not show up in any tests.
>>
>>> +
>>> +               buf = io_buffer_select(req, &sr->len, sr->bgid,
>>> issue_flags);
>>> +               if (IS_ERR(buf))
>>> +                       return PTR_ERR(buf);
>>>         }
>>>  
>>>         ret = import_single_range(READ, buf, sr->len, &iov,
>>> &msg.msg_iter);
>>
> 
> The following seems to fix it for me. I can submit it separately if you
> like.

I think you want something like this:


diff --git a/fs/io_uring.c b/fs/io_uring.c
index 19dd3ba92486..2b87c89d2375 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5599,7 +5599,6 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
-	void __user *buf = sr->buf;
 	struct socket *sock;
 	struct iovec iov;
 	unsigned flags;
@@ -5620,9 +5619,10 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 		buf = io_buffer_select(req, &sr->len, sr->bgid, issue_flags);
 		if (IS_ERR(buf))
 			return PTR_ERR(buf);
+		sr->buf = buf;
 	}
 
-	ret = import_single_range(READ, buf, sr->len, &iov, &msg.msg_iter);
+	ret = import_single_range(READ, sr->buf, sr->len, &iov, &msg.msg_iter);
 	if (unlikely(ret))
 		goto out_free;
 

-- 
Jens Axboe