All of lore.kernel.org
 help / color / mirror / Atom feed
* io_uring stable addition
@ 2019-12-04 15:53 Jens Axboe
  2019-12-07 12:01 ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2019-12-04 15:53 UTC (permalink / raw)
  To: stable

Hi,

We have an issue with drains not working due to missing copy of some
state, it's affecting 5.2/5.3/5.4. I'm attaching the patch for 5.4,
however the patch should apply to 5.2 and 5.3 as well by just removing
the last hunk. The last hunk is touching the linked code, which was
introduced with 5.4.

Can we get this queued up for stable? Thanks! Don't have an email for
Tomáš, assuming the reported-by is fine with just his name. Want to
ensure I include attribution I do have.


From: Jens Axboe <axboe@kernel.dk>
Subject: [PATCH] io_uring: ensure req->submit is copied when req is deferred

There's an issue with deferred requests through drain, where if we do
need to defer, we're not copying over the sqe_submit state correctly.
This can result in using uninitialized data when we then later go and
submit the deferred request, like this check in __io_submit_sqe():

         if (unlikely(s->index >= ctx->sq_entries))
                 return -EINVAL;

with 's' being uninitialized, we can randomly fail this check. Fix this
by copying sqe_submit state when we defer a request.

Reported-by: Andres Freund <andres@anarazel.de>
Reported-by: Tomáš Chaloupka
Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 2c819c3c855d..0393545a39a7 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2016,7 +2017,7 @@ static int io_timeout(struct io_kiocb *req, const struct io_uring_sqe *sqe)
  }
  
  static int io_req_defer(struct io_ring_ctx *ctx, struct io_kiocb *req,
-			const struct io_uring_sqe *sqe)
+			struct sqe_submit *s)
  {
  	struct io_uring_sqe *sqe_copy;
  
@@ -2034,7 +2035,8 @@ static int io_req_defer(struct io_ring_ctx *ctx, struct io_kiocb *req,
  		return 0;
  	}
  
-	memcpy(sqe_copy, sqe, sizeof(*sqe_copy));
+	memcpy(&req->submit, s, sizeof(*s));
+	memcpy(sqe_copy, s->sqe, sizeof(*sqe_copy));
  	req->submit.sqe = sqe_copy;
  
  	INIT_WORK(&req->work, io_sq_wq_submit_work);
@@ -2399,7 +2401,7 @@ static int io_queue_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
  {
  	int ret;
  
-	ret = io_req_defer(ctx, req, s->sqe);
+	ret = io_req_defer(ctx, req, s);
  	if (ret) {
  		if (ret != -EIOCBQUEUED) {
  			io_free_req(req);
@@ -2426,7 +2428,7 @@ static int io_queue_link_head(struct io_ring_ctx *ctx, struct io_kiocb *req,
  	 * list.
  	 */
  	req->flags |= REQ_F_IO_DRAIN;
-	ret = io_req_defer(ctx, req, s->sqe);
+	ret = io_req_defer(ctx, req, s);
  	if (ret) {
  		if (ret != -EIOCBQUEUED) {
  			io_free_req(req);

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: io_uring stable addition
  2019-12-04 15:53 io_uring stable addition Jens Axboe
@ 2019-12-07 12:01 ` Greg KH
  2019-12-07 15:19   ` Jens Axboe
  0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2019-12-07 12:01 UTC (permalink / raw)
  To: Jens Axboe; +Cc: stable

On Wed, Dec 04, 2019 at 08:53:43AM -0700, Jens Axboe wrote:
> Hi,
> 
> We have an issue with drains not working due to missing copy of some
> state, it's affecting 5.2/5.3/5.4. I'm attaching the patch for 5.4,
> however the patch should apply to 5.2 and 5.3 as well by just removing
> the last hunk. The last hunk is touching the linked code, which was
> introduced with 5.4.

Does this match up with a patch in Linus's tree anywhere?  Why is this a
stable-only thing?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: io_uring stable addition
  2019-12-07 12:01 ` Greg KH
@ 2019-12-07 15:19   ` Jens Axboe
  2019-12-07 15:34     ` Greg KH
  0 siblings, 1 reply; 4+ messages in thread
From: Jens Axboe @ 2019-12-07 15:19 UTC (permalink / raw)
  To: Greg KH; +Cc: stable

On 12/7/19 5:01 AM, Greg KH wrote:
> On Wed, Dec 04, 2019 at 08:53:43AM -0700, Jens Axboe wrote:
>> Hi,
>>
>> We have an issue with drains not working due to missing copy of some
>> state, it's affecting 5.2/5.3/5.4. I'm attaching the patch for 5.4,
>> however the patch should apply to 5.2 and 5.3 as well by just removing
>> the last hunk. The last hunk is touching the linked code, which was
>> introduced with 5.4.
> 
> Does this match up with a patch in Linus's tree anywhere?  Why is this a
> stable-only thing?

Because it was fixed as part of a cleanup series in mainline, before
anyone realized we had this issue. That removed the separate states
of ->index vs ->submit.sqe. That series is not something I was
comfortable putting into stable, hence the much simpler addition.
Here's the patch in the series that fixes the same issue:

commit cf6fd4bd559ee61a4454b161863c8de6f30f8dca
Author: Pavel Begunkov <asml.silence@gmail.com>
Date:   Mon Nov 25 23:14:39 2019 +0300

    io_uring: inline struct sqe_submit


-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: io_uring stable addition
  2019-12-07 15:19   ` Jens Axboe
@ 2019-12-07 15:34     ` Greg KH
  0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2019-12-07 15:34 UTC (permalink / raw)
  To: Jens Axboe; +Cc: stable

On Sat, Dec 07, 2019 at 08:19:18AM -0700, Jens Axboe wrote:
> On 12/7/19 5:01 AM, Greg KH wrote:
> > On Wed, Dec 04, 2019 at 08:53:43AM -0700, Jens Axboe wrote:
> >> Hi,
> >>
> >> We have an issue with drains not working due to missing copy of some
> >> state, it's affecting 5.2/5.3/5.4. I'm attaching the patch for 5.4,
> >> however the patch should apply to 5.2 and 5.3 as well by just removing
> >> the last hunk. The last hunk is touching the linked code, which was
> >> introduced with 5.4.
> > 
> > Does this match up with a patch in Linus's tree anywhere?  Why is this a
> > stable-only thing?
> 
> Because it was fixed as part of a cleanup series in mainline, before
> anyone realized we had this issue. That removed the separate states
> of ->index vs ->submit.sqe. That series is not something I was
> comfortable putting into stable, hence the much simpler addition.
> Here's the patch in the series that fixes the same issue:
> 
> commit cf6fd4bd559ee61a4454b161863c8de6f30f8dca
> Author: Pavel Begunkov <asml.silence@gmail.com>
> Date:   Mon Nov 25 23:14:39 2019 +0300
> 
>     io_uring: inline struct sqe_submit

Thanks for the info, now queued up!

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-12-07 15:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 15:53 io_uring stable addition Jens Axboe
2019-12-07 12:01 ` Greg KH
2019-12-07 15:19   ` Jens Axboe
2019-12-07 15:34     ` Greg KH

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.